Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement multiple label #1785

Closed

Conversation

rafsun42
Copy link
Member

@rafsun42 rafsun42 commented Apr 24, 2024

Add support for multiple labels in CREATE, MERGE and MATCH clause.

Important Notes:

  • Before merging, please rerun the workflow again. Any tests that were added after this branch was last updated will fail because this PR makes minor changes in how vertex and edge objects are output.

Update:

  • Base of this PR is changed to a new remote branch multiple_label_16, which is based off master.

Copy link
Contributor

@Zainab-Saad Zainab-Saad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, added comments for some minor issues.

src/backend/parser/cypher_label_expr.c Show resolved Hide resolved
ListCell *lc;
char *relname;

Assert(list_length(label_expr->label_names) > 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CREATE (x:A:A) RETURN x results in server termination at this Assertion..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! I remember you reported this one before. I will fix it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this in my backlog.

@dehowef
Copy link
Member

dehowef commented May 1, 2024

@rafsun42 Looks good to me-- this seems to be a really big change seeing that it affects the regression tests in a large way. I think it should be a priority to get this reviewed by as many people as possible, so that development can continue. @jrgemignani mind taking a look?

@MuhammadTahaNaveed MuhammadTahaNaveed added the override-stale To keep issues/PRs untouched from stale action label May 9, 2024
It represents label expression of different type: empty, single or multiple.
Previously, label field was char* type.

The change affected the type cypher_node, cypher_relationship
and cypher_target_node. As well as, any places where these
types are used.
Supports queries like-
    MATCH (v:A|B|C) RETURN v
    MATCH ()-[e:A|B|C]->() RETURN v
Some examples of supported multiple label queries:
	CREATE (:a:b)
	MERGE  (:a:b)
	MATCH  (:a:b)
	MATCH  (:a|b)

See regress/sql/multiple_label.sql for more details on what kind
of queries are supported.

Change summary:
---------------
* A new column `allrelations` is added to ag_label catalog
* Change in creating AGE relations logic
* Change in MATCH's transformation logic (related to building parse
  namespace item)
The logic for building vertex objects is updated. Agtype vertex objects can be
built from either a single label (as a cstring) or multiple labels (as an
agtype array). The following functions are updated to reflect this-
agtype_typecast_vertex, agtype_in and _agtype_build_vertex. if
_agtype_build_vertex is called from SQL, its label argument must be explicitly
cast to avoid ambiguity in function overload.

The `_label_names` function is added to extract label names from a vertex ID
as a list of string. It is used as a helper function to build vertex objects.
A new cache called `allrelations` is also added. This is used by _label_names
to search for all labels that are related to a given relation.

Multiple helper functions are added to extract label infromation from an entity
ID. For example, entity's relation ID, relation name, label names. These are
used by CREATE, DELETE, MERGE, VLE and SET executors for building a vertex's
object or updating its relation.

All test files are updated to show the label field as an array in the output.
In all test SQLs, _agtype_build_vertex's label argument is explicity cast.
It updates the function filter_vertices_on_label_id().

Additional changes:
-------------------
 - Add internal function _label_ids
Cache issues fixed:
-------------------
  - Use of wrong data type for cache entry in label relation cache (pre-existing)
  - Use of wrong update function for catalog table (related to multiple label)

Other changes:
--------------
  - The function _label_name() is unsupported for vertices
Changes:
--------
 - Update create_label_expr_relations() to return RangeVar. It removes
   redundant call to label_expr_relname() in the code that also calls
   this function.

 - Use deconstruct_array() to convert ArrayType* to List*

 - Update test files after rebase
This fixes some compile-time errors that occur if
PostgreSQL is configured with the --with-llvm option.
Changes:
-------
  * Include missing header files
  * Update newly added tests
  * Other minor changes
Following PRs are reapplied: 1465, 1509, 1514, and 1518.
@rafsun42 rafsun42 changed the base branch from master to multiple_label_16 August 28, 2024 17:58
@rafsun42
Copy link
Member Author

@jrgemignani @MuhammadTahaNaveed This PR is ready for review now. It was rebased to a new remote branch multiple_label_16, which is based off master. So, there won't be any merge conflicts when changes are added to master. Once it is merged, the remote multiple_label_16 needs to be rebased and merged to master.

@rafsun42 rafsun42 changed the base branch from multiple_label_16 to master August 29, 2024 19:27
@rafsun42 rafsun42 changed the base branch from master to multiple_label_16 August 29, 2024 19:27
@rafsun42 rafsun42 closed this Aug 29, 2024
@rafsun42 rafsun42 reopened this Aug 29, 2024
* Regression tests
* Comments
* Minor overlook during rebasing
@rafsun42
Copy link
Member Author

Moved to #2082

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master override-stale To keep issues/PRs untouched from stale action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants