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

Fix Issue 1329 - agtype_to_int4 crash #1339

Merged
merged 1 commit into from Nov 1, 2023

Conversation

jrgemignani
Copy link
Contributor

@jrgemignani jrgemignani commented Oct 31, 2023

Fixed issue 1329 where agtype_to_int<8,4,2> and agtype_to_int4_array crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly checked before casting it to a specific type. The input section assumed it was agtype, which caused crashes for non-agtypes.

The functions agtype_to_int<8,4,2> will convert non-agtypes into agtype. However, there were no regression tests for this.

The functions agtype_to_int<8,4,2> will convert non-agtypes to agtype ints but, did not for their string equivs. Meaning, passing a ('true') or ('3.14') would fail but, passing a (true) or (3.14) would not. This has been corrected for all 3 functions.

TODO -
The function agtype_to_int4_array only takes agtype, currently, and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.

Copy link
Member

@MuhammadTahaNaveed MuhammadTahaNaveed left a comment

Choose a reason for hiding this comment

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

Lgtm

@jrgemignani
Copy link
Contributor Author

@MuhammadTahaNaveed @rafsun42 @dehowef Please recheck as I have updated this PR.

Copy link
Member

@dehowef dehowef 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 to me

Int64GetDatum(agtv_p->val.int_value)));
}
else if (agtv_p->type == AGTV_FLOAT)
{
result = DatumGetInt32(DirectFunctionCall1(dtoi2,
Copy link
Member

Choose a reason for hiding this comment

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

Should this be DatumGetInt16?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it looks like this was an artifact from the original code.

Fixed issue 1329 where `agtype_to_int`<8,4,2> and `agtype_to_int4_array`
crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly
checked before casting it to a specific type. The input section
assumed it was agtype, which caused crashes for non-agtypes.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes into
agtype. However, there were no regression tests for this.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes to
agtype ints but, did not for their string equivs. Meaning, passing
a ('true') or ('3.14') would fail but, passing a (true) or (3.14)
would not. This has been corrected for all 3 functions.

TODO -
The function `agtype_to_int4_array` only takes agtype, currently,
and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.
@rafsun42 rafsun42 merged commit b25f6f7 into apache:master Nov 1, 2023
7 checks passed
jrgemignani added a commit to jrgemignani/age that referenced this pull request Nov 1, 2023
Fixed issue 1329 where `agtype_to_int`<8,4,2> and `agtype_to_int4_array`
crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly
checked before casting it to a specific type. The input section
assumed it was agtype, which caused crashes for non-agtypes.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes into
agtype. However, there were no regression tests for this.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes to
agtype ints but, did not for their string equivs. Meaning, passing
a ('true') or ('3.14') would fail but, passing a (true) or (3.14)
would not. This has been corrected for all 3 functions.

TODO -
The function `agtype_to_int4_array` only takes agtype, currently,
and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.
jrgemignani added a commit to jrgemignani/age that referenced this pull request Nov 2, 2023
Fixed issue 1329 where `agtype_to_int`<8,4,2> and `agtype_to_int4_array`
crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly
checked before casting it to a specific type. The input section
assumed it was agtype, which caused crashes for non-agtypes.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes into
agtype. However, there were no regression tests for this.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes to
agtype ints but, did not for their string equivs. Meaning, passing
a ('true') or ('3.14') would fail but, passing a (true) or (3.14)
would not. This has been corrected for all 3 functions.

TODO -
The function `agtype_to_int4_array` only takes agtype, currently,
and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.
jrgemignani added a commit to jrgemignani/age that referenced this pull request Nov 2, 2023
Fixed issue 1329 where `agtype_to_int`<8,4,2> and `agtype_to_int4_array`
crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly
checked before casting it to a specific type. The input section
assumed it was agtype, which caused crashes for non-agtypes.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes into
agtype. However, there were no regression tests for this.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes to
agtype ints but, did not for their string equivs. Meaning, passing
a ('true') or ('3.14') would fail but, passing a (true) or (3.14)
would not. This has been corrected for all 3 functions.

TODO -
The function `agtype_to_int4_array` only takes agtype, currently,
and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.
jrgemignani added a commit to jrgemignani/age that referenced this pull request Nov 2, 2023
Fixed issue 1329 where `agtype_to_int`<8,4,2> and `agtype_to_int4_array`
crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly
checked before casting it to a specific type. The input section
assumed it was agtype, which caused crashes for non-agtypes.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes into
agtype. However, there were no regression tests for this.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes to
agtype ints but, did not for their string equivs. Meaning, passing
a ('true') or ('3.14') would fail but, passing a (true) or (3.14)
would not. This has been corrected for all 3 functions.

TODO -
The function `agtype_to_int4_array` only takes agtype, currently,
and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.

Conflicts:
	regress/expected/agtype.out
	regress/expected/expr.out
	src/backend/utils/adt/agtype.c
rafsun42 pushed a commit that referenced this pull request Nov 3, 2023
Fixed issue 1329 where `agtype_to_int`<8,4,2> and `agtype_to_int4_array`
crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly
checked before casting it to a specific type. The input section
assumed it was agtype, which caused crashes for non-agtypes.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes into
agtype. However, there were no regression tests for this.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes to
agtype ints but, did not for their string equivs. Meaning, passing
a ('true') or ('3.14') would fail but, passing a (true) or (3.14)
would not. This has been corrected for all 3 functions.

TODO -
The function `agtype_to_int4_array` only takes agtype, currently,
and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.

Conflicts:
	regress/expected/agtype.out
	regress/expected/expr.out
	src/backend/utils/adt/agtype.c
rafsun42 pushed a commit that referenced this pull request Nov 3, 2023
Fixed issue 1329 where `agtype_to_int`<8,4,2> and `agtype_to_int4_array`
crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly
checked before casting it to a specific type. The input section
assumed it was agtype, which caused crashes for non-agtypes.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes into
agtype. However, there were no regression tests for this.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes to
agtype ints but, did not for their string equivs. Meaning, passing
a ('true') or ('3.14') would fail but, passing a (true) or (3.14)
would not. This has been corrected for all 3 functions.

TODO -
The function `agtype_to_int4_array` only takes agtype, currently,
and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.
rafsun42 pushed a commit that referenced this pull request Nov 3, 2023
Fixed issue 1329 where `agtype_to_int`<8,4,2> and `agtype_to_int4_array`
crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly
checked before casting it to a specific type. The input section
assumed it was agtype, which caused crashes for non-agtypes.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes into
agtype. However, there were no regression tests for this.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes to
agtype ints but, did not for their string equivs. Meaning, passing
a ('true') or ('3.14') would fail but, passing a (true) or (3.14)
would not. This has been corrected for all 3 functions.

TODO -
The function `agtype_to_int4_array` only takes agtype, currently,
and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.
rafsun42 pushed a commit that referenced this pull request Nov 3, 2023
Fixed issue 1329 where `agtype_to_int`<8,4,2> and `agtype_to_int4_array`
crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly
checked before casting it to a specific type. The input section
assumed it was agtype, which caused crashes for non-agtypes.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes into
agtype. However, there were no regression tests for this.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes to
agtype ints but, did not for their string equivs. Meaning, passing
a ('true') or ('3.14') would fail but, passing a (true) or (3.14)
would not. This has been corrected for all 3 functions.

TODO -
The function `agtype_to_int4_array` only takes agtype, currently,
and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.
jrgemignani added a commit to jrgemignani/age that referenced this pull request Nov 16, 2023
Fixed issue apache#1339 where EXISTS doesn't handle non-existent labels.
However, the issue actually is due to the extra processing (read:
extra distance between the AST nodes) that prevented the transform
logic from being able to resolve variables created in previous clauses
further than one parent up.

Added logic to allow the `find_variable` function to search up the
parent parse nodes.

Added regression tests to cover these cases.
jrgemignani added a commit that referenced this pull request Dec 13, 2023
Fixed issue 1329 where `agtype_to_int`<8,4,2> and `agtype_to_int4_array`
crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly
checked before casting it to a specific type. The input section
assumed it was agtype, which caused crashes for non-agtypes.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes into
agtype. However, there were no regression tests for this.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes to
agtype ints but, did not for their string equivs. Meaning, passing
a ('true') or ('3.14') would fail but, passing a (true) or (3.14)
would not. This has been corrected for all 3 functions.

TODO -
The function `agtype_to_int4_array` only takes agtype, currently,
and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.
Zainab-Saad pushed a commit to Zainab-Saad/age that referenced this pull request Mar 6, 2024
Fixed issue 1329 where `agtype_to_int`<8,4,2> and `agtype_to_int4_array`
crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly
checked before casting it to a specific type. The input section
assumed it was agtype, which caused crashes for non-agtypes.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes into
agtype. However, there were no regression tests for this.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes to
agtype ints but, did not for their string equivs. Meaning, passing
a ('true') or ('3.14') would fail but, passing a (true) or (3.14)
would not. This has been corrected for all 3 functions.

TODO -
The function `agtype_to_int4_array` only takes agtype, currently,
and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.
WendelLana added a commit to WendelLana/age that referenced this pull request Apr 17, 2024
Implemented integer conversion to boolean in both toBoolean and
toBooleanList functions. Also made corresponding changes to regression
tests and broke long lines to adhere to the 79-column line length limit
in the toBooleanList function.

Minor fix in `agtype_volatile_wrapper` function (apache#1172)

Fixed duplicated IF-STATEMENT of INT4OID in agtype_volatile_wrapper
function changing it to INT2OID

Update files for Apache AGE release 1.4.0 (apache#1194)

Changes to be committed:

    modified:   Makefile
    modified:   README.md
    modified:   RELEASE
    renamed:    age--1.3.0.sql -> age--1.4.0.sql
    modified:   age.control

Removed unnecessary assignment (apache#1185)

Original work was done by Panagiotis Foliadis in PR apache#1035.

Co-authored-by: Panagiotis Foliadis <pfoliadis@hotmail.com>

Fix Issue#1159: Server terminates for SET plus-equal (apache#1160)

The previous implementation of alter_properties() in
agtype.c while copying the original properties ignored
non-scalar value cases, this PR fixes that

Add concat || operator to agtype (apache#1198)

- Implement the concatenation operator for agtype as operands, similar
  to the one in postgres where this operator works with jsonb operands

- Allow using concat operator inside cypher queries

- Add jsonb_operators.sql and jsonb_operators.out files
  for containing regression tests related to jsonb operators,
  i.e., (?,?|,?&,->,->>,#>,#>>,||)

Add exist(?, ?|, ?&) operators for agtype (apache#1218)

- Added exist(?, ?|, ?&) operators for agtype similar to jsonb operators
  in postgresql.
- Allow exist(?, ?|, ?&) operators to be used inside cypher queries.
- Added regression tests.

Add additional index support and performance enhancements (apache#1232)

Added additional index support for the WHERE clause. This work was
partly from another patch authored by Josh Innis.

Changed the volatility flag to IMMUTABLE for many functions related
to indexes. I.e. comparison and build functions.

Updated _agtype_build_path to be IMMUTABLE.

No regression tests were impacted by these changes.

Add path extraction(#>, #>>)operators to agtype (apache#1224)

Original work was done by Josh Innis for issue#282.

The changes included in this PR are as follows:

- Implement the path extraction operators for agtype, similar
  to the one in postgres where this operator works with jsonb operands
- Allow using these operators inside cypher queries
- Added regression tests

Update the Python Driver (apache#1246)

- Updated the README contents of the Python driver for the pypi
release.
- Fixed errors in test_age_py.py that was causing connectivity issues.

Refactor the IN operator to use '= ANY()' syntax (apache#1236)

This change reduces use of internal function calls from the
IN implementation to optimize performance. This also changes
IN behavior to correctly return NULL upon NULL elements
included in the list that IN checks against.

Added and corrected regression tests.

Co-authored by: Josh Innis <JoshInnis@gmail.com>

Update Discord channel in README.md (apache#1253)

Add missing dependency in cypher_expr.c (apache#1256)

The function contain_vars_of_level() was missing a header.
Added that header to prevent a warning during compilation.

Extend access(->, ->>), addition and subtraction operators (apache#1258)

Original work was done by Josh Innis for issue#282.

The changes included in this PR are as follows:

- Enable the use of access operator with agtype as the RHS operand,
  allowing this operator to be used within cypher queries
- Extend addition operator to concatenate in case any or both
  operands are non-scalar or scalar vertex/edge/path
- Extend subtract operator to delete specified keys from object or
  elements at specified indexes from array
- Add and modify regression tests relevant to the above changes

Optimize performance of detach delete (apache#1271)

Previously, for each vertex to be deleted, all edge tables were
scanned once to process the connected edges. Now, this task is postponed
until all vertices are deleted. So, the connected edges can be processed
in only one scan of the edge tables regardless of the number of deleted
vertices.

Refactor Regression Tests for CASE statement (apache#1268)

Added some new cases to CASE statement regression tests to capture
cases that were not tested for before.

Refactored existing test cases to improve readability

Extend EXPLAIN and add config param to switch transformation of property filter (apache#1262)

* Add explain options

Explain command in the following format is supported
now:
    `EXPLAIN (VERBOSE, COSTS OFF, FORMAT XML) ...`

Note that, this is basically Postgres' EXPLAIN command,
and the purpose of this is to support debugging and
regression tests.

* Add config param to switch transformation method of property filter

When the `age.enable_containment` parameter is on, the agtype
containment operator is used to transform property filter. When off,
access operator is used instead. The former case is preferable for
GIN index, and the later for BTREE expression index.

The idea of replacing containment with access operator in order to
support BTREE index is taken from a patch by Josh Innis.

A note on regression testing- although there are test cases for the
`age.enable_containment` parameter, sometimes it may be useful to
set this parameter before running any tests (not just the ones related
to it). For example, when the logic related to property transformation
changes. It can be set in the `age_regression.conf` file.

Implemented age_tail function (apache#1283)

age_tail() function returns a list containing all the elements,
excluding the first one, from a list.

Optmize vertex and edge builder functions (apache#1252)

Changes:
    - Added the agtype_raw module, which contains helper functions to
      build agtype directly without building an agtype_value first
    - Optimize _agtype_build_vertex and _agtype_build_edge functions

The agtype_raw module:
    Inserting a composite agtype (i.e. object) into another agtype
    (i.e. as an array element) requires the first agtype to be deserialized
    into agtype_value. Then, the agtype_value to be serialized back into
    the second agtype.

    This module provides functions that can perform such insertion without
    deserializing first. It is meant to speed up queries that does
    deserialization-serialization back and forth involving deeply nested
    agtype objects.

docs: Add to Docker setup (apache#1204)

- add command to enter psql interface from command line.

Fix Issue 945 - incorrect count(*) return values (apache#1288)

Fixed issue 945 where count(*) would have incorrect return values.

NOTE -

We need to re-evaluate if we want to use output_node or not.
If output_node is set to false, then it basically short circuits
match for instances where a variable isn't specified -

    MATCH () RETURN 0;
    MATCH () MATCH () RETURN 0;

While, on the surface, this appears to be a good way to improve
execution time of commands that won't do anything, it also
causes chained commands to not work correctly. This is because
a match without a variable will still feed its tuples to the next
stage(s) even though they won't necessarily be sent to the output.
For example, with count(*) -

    MATCH () RETURN count(*);
    MATCH () MATCH RETURN count(*);

With output_node set to false, it won't send the tuples. We will
likely need to remove all of the output_node logic. However, this
needs to be reviewed. For now, we just set it to true and update
the output of the regression tests.

Updated regression tests to accomodate the change.
Added new regression tests to cover overlooked cases.

Fix issue apache#1302 - crash on NULL input to UNWIND (apache#1304)

- Added a check for NULL input in age_unnest.
- Added regression tests.

Fix Issue 1305 - drop_label NULL cases (apache#1306)

Fixed issue 1305 where drop_label doesn't cover the case of rel_name
coming back NULL, meaning it wasn't found.

Additionally, added a fix for when the schema_name isn't found from
the namespace id search.

Added regression tests

Fix issue 1303: Server crashes on executing SELECT * FROM agtype(null); (apache#1317)

* Add additional checks for Const expression nodes for which the
      walker code would crash.

    * The server crashes for few other expr types too including Const,
      OpExpr, Var, BoolExpr and CoerceViaIO for which checks are added

    * Add regression tests

Extend agtype containment operators (@>, <@) (apache#1285)

This PR is part of the patch originally authored by Josh Innis
 for issue # 282. The work included in this PR is as follows:

 - Enable the usage of left and right containment operators inside
   the cypher queries
 - Extend this operator to be used with scalars
 - Add relevant regression tests

Fix issue apache#1045 - error using path var in WHERE (apache#1295)

- Added cypher_path as an entity(along side cypher_node and
  cypher_relationship) in order for WHERE in MATCH clause to
  be able to reference it.

- Fixed relevant error messages to be more specific.

- Added regression tests.

Modify COUNT() to output agtype (apache#1311)

Modified the make_function_expr logic to wrap the PG COUNT()
function with a cast to agtype. This enables COUNT() to be used as
a subquery in CASE.

Also added logic for casting for future PG function
additions.

This modification passes all regression tests. Also added
regression tests for COUNT in CASE statements.

Fix Issue 1329 - agtype_to_int4 crash (apache#1339)

Fixed issue 1329 where `agtype_to_int`<8,4,2> and `agtype_to_int4_array`
crashed due to not properly checking input.

As these functions take "any" input, the input has to be properly
checked before casting it to a specific type. The input section
assumed it was agtype, which caused crashes for non-agtypes.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes into
agtype. However, there were no regression tests for this.

The functions `agtype_to_int`<8,4,2> will convert non-agtypes to
agtype ints but, did not for their string equivs. Meaning, passing
a ('true') or ('3.14') would fail but, passing a (true) or (3.14)
would not. This has been corrected for all 3 functions.

TODO -
The function `agtype_to_int4_array` only takes agtype, currently,
and we should consider allowing it to take "any" types.

Added regression tests.
Added missing regression tests.

Fix issue apache#1347 - unknow type of agtype container 0 (apache#1349)

The error was caused by the Datum referenced by
edge_entry->edge_properties becoming non-persistent. As a
result, the pointer becomes dangling. This issue is fixed
by making a copy of the Datum into the MemoryContext it is
used. The fix is also applied to
vertex_entry->vertex_properties.

Thanks to John for finding out the correct function to
copy a Datum.

Co-authored-by: John Gemignani <jrgemignani@gmail.com>

Clean up agtype_to_int8, agtype_to_int4, & agtype_to_int2 (apache#1354)

Cleaned up the code in `agtype_to_int8`, `agtype_to_int4`, and
`agtype_to_int2`. There was some dead code after implementing
PR 1339.

Additionally, modified the error messages to be a bit more helpful
and modified the logic to cover a few more cases.

Added more regression tests.

Fix typo in agtype_raw.h header guard (apache#1368)

Header guard AG_AGTYPE_RAW_H was misspelled AG_AGTYPE_RAw_H

Implement EXISTS subquery for CASE (apache#1345)

Modified logic of EXISTS to wrap itself in a agtype cast. This
allows EXISTS to work with in CASE statements, among other
implementations that may require an agtype.

Also renamed the wrapper function to be more generic to denote
its usage in other implementations if need be.

Added regression tests for EXISTS

Fix ambiguous conditions (apache#1373)

- These conditions were ambiguous for compiler as well as for other
  devs.

Fix DockerHub warning messages for latest (apache#1380)

Fixed DockerHub warning messages for the build 'latest'. These were
due to Assert statements that used variables that were not used for
anything else. I changed them to ifs with error log outputs.

Fix issue apache#1389 - Server crash on using null operand for access operators (apache#1390)

- The issue was due to code not handling null values as LHS operands for
  access operators(->, ->>)

  Added check for this case and included regression tests

Remove unnecessary #include in src/backend/utils

Removed unnecessary #includes in src/backend/utils files.

Fix issue apache#1399 EXISTS doesn't handle non-existent labels (apache#1400)

Fixed issue apache#1399 where EXISTS doesn't handle non-existent labels.
However, the issue actually is due to the extra processing (read:
extra distance between the AST nodes) that prevented the transform
logic from being able to resolve variables created in previous clauses
further than one parent up.

Added logic to allow the `find_variable` function to search up the
parent parse nodes.

Added regression tests to cover these cases.

Implement chained expression order of operations (apache#1402)

Implements functionality for order of operations (parentheses) in
chained operations.

Added new nodes and functions to accomodate this feature.

Added relevant regression tests for order of operations in chained
expressions.

Modified a previous regression test case that had its location
reporting changed by this implementation.

Converted SQL mail into multiple files. (apache#1401)

Converted SQL mail into multiple files.

This pull request is aimed at improving the overall quality of our SQL codebase
by converting the main SQL (age-x.x.x.sql) file into multiple smaller files in the
sql directory. The primary goal is to enhance code readability, maintainability,
and organization for the benefit of the development team.

Fix issue apache#1398 - SET followed by DELETE does not delete (apache#1412)

In the DELETE executor, tuples were being locked\deleted using the command ID
returned by GetCurrentCommandId(). However, the original tuples were retrieved
using a command ID stored in the estate.

To fix it- before the retrieval of a tuple, estate command IDs are set using
GetCurrentCommandId() so it matches if the tuple is to be deleted later.

Additional changes:
-------------------
Fixed an incorrect cypher_delete test. The query
`MATCH (n1)-[e]->() DELETE n1, e RETURN n1` should be able to delete n1 and e
without requiring DETACH DELETE.

TODO:
-----
It may be a good idea to audit the executors for any inconsistent use of
command IDs.

Fix issue 1393 - previous clause variables not seen with EXISTS (apache#1426)

Fixed issue 1393 - Unexpected error when matching nodes that are
connected to nodes resulted from a previous MATCH clause.

Basically, variables from the previous clauses where not seen when
using the EXISTS clause for path pattern matches. This only affected
EXISTS and only for the EXISTS (anonymous_path) grammar component.

The original issue was only with vertices, but this also affected
edges. Both cases are now fixed.

Fixed 1 regression test that was incorrect.
Added regression tests.

Add support for chained expressions in CASE (apache#1431)

Adds logic that implements CASE in chained expressions.

Adds regression tests for CASE chained expression logic.

Fix issue 1219 - MERGE not seeing previous clause var (apache#1441)

Fixed issue 1219 where MERGE did not see the previous clause's
variable.

This description is a bit misleading as the transform phase did see
the variable and was able to use it. However, the planner phase
removed the variable by replacing it with a NULL Const. This caused
MERGE to see a NULL Const for the previous tuple, generating
incorrect results. However, this only occurred for very specific
cases.

Fx:    MATCH (x) MERGE (y {id: id(x)})              -- worked
       MATCH (x) MERGE (y {id: id(x)}) RETURN y     -- didn't
       MATCH (x) MERGE (y {id: id(x)}) RETURN x, y  -- worked

The change impacted no current regression tests and involved wrapping
all explicitly defined variables' target entries with a volatile
wrapper.

Added new regression tests.

Fix Docker file to reflect PostgreSQL version 15 (apache#1449)

Fixed the following Docker file to reflect running under
PostgreSQL version 15.

modified:   docker/Dockerfile.dev

This impacts the development DockerHub builds.

Master to PostgreSQL version 16 (apache#1451)

* Initial PG16 version (apache#1237)

Fixed empty string handling.

Previously, the PG 16 outToken emitted NULL for an empty string, but now it emits an empty string "". Consequently, our cypher read function required modification to properly decode the empty string as a plain token, thereby addressing the comparison issue.

Compared the branches and added the necessary changes so that the query's rteperminfos variable doesn't stay NULL.

Fix missing include varatt.h (causing undefined symbol VARDATA_ANY) & Fixing some test cases of the failings

Added missing include varatt.h to fix the undefined symbol while loading age into postgresql because usage of VARDATA_ANY needs to import varatt.h in PG16

Modified initialisation of ResultRelInfo and removed unnecessary RTEs

Compared the branches and added the necessary changes so that the query's rteperminfos variable doesn't stay NULL.

Modified initialisation of ResultRelInfo and removed unnecessary RTEs

One of the problems that we were facing was related to the ResultRelInfo pointing at the wrong RTE via its ri_RangeTableIndex. The create_entity_result_rel_info() function does not have the capability of setting the ri_RootResultRelInfo to the correct ResultRelInfo node because it does not have access to the ModifyTableState node. The solution for this was to set the third argument in InitResultRelInfo() to be zero instead of list_length(estate->es_range_table).

In the update_entity_tuple() function, when we call table_tuple_update() and assign the returned value to the result variable, the buffer variable receives the value of 0.
Made a workaround so that the original value isn't lost.

This is a work in progress for the new field that was added to the struct Var called varnullingrels. According to the documentation, this field is responsible for marking the Vars as nullable, if they are coming from a JOIN, either LEFT JOIN, RIGHT JOIN, or FULL OUTER JOIN. The changes were made following an "optional match" clause which is being treated as a LEFT JOIN from our extension.

A function markRelsAsNulledBy is added because its internal in Postgres and doesn't belong in a header file, therefore it can't be exported. This function is added before the creation of the Vars from the make_vertex_expr and make_edge_expr, to correctly mark the specific PNSI as nullable, so later in the planner stage, the Vars will be correctly nulled.

Fix incorrect typecasting in agtype_to_graphid function.

Fix incorrect returns to the fuction _label_name, _ag_build_vertex and _ag_build_edge.
Contributors

Panagiotis Foliadis <pfoliadis@hotmail.com>
Matheus Farias <matheusfarias519@gmail.com>
Mohamed Mokhtar <m.mokhtar@outlook.it>
Hannan Aamir <hannanaamir2000@gmail.com>
John Gemignani <jrgemignani@gmail.com>
Muhammad Taha Naveed <m.taha.naveed27@gmail.com>
Wendel de Lana <sr.wendelfernandes@outlook.com>
---------

* Fix Docker files to reflect PostgreSQL version 16 (apache#1448)

Fixed the following Docker files to reflect running under
PostgreSQL version 16.

    modified:   docker/Dockerfile
    modified:   docker/Dockerfile.dev

This impacts the DockerHub builds.

* Mark null-returning RTEs in outer joins as nullable

RTEs that appear in the right side of a left join are marked as 'nullable'.
The column references to a nullable RTE within the join's RTE are also
marked as 'nullable'. This concept is introduced in Postgresql v16.

The change in Postgresql v16's pullup_replace_vars_callback() function causes
different plans to be generated depending on whether appropriate RTEs are
marked nullable. Without marking nullable, in a left join, any function call
expression containing right RTE's columns as arguments are not evaluated at
the scan level, rather it is evaluated after the join is performed. At that
point, the function call may receive null input, which was unexpected in
previous Postgresql versions.

See:
----
  - Postgresql v16's commit: Make Vars be outer-join-aware
    https://www.postgresql.org/message-id/830269.1656693747@sss.pgh.pa.us

  - The 'Vars and PlaceHolderVars' section in the Postgresql v16's
    optimizer/README.md

* Fix minor code formatting.

---------

Co-authored-by: Shoaib <muhemmed.shoaib@gmail.com>
Co-authored-by: Rafsun Masud <rafsun82@gmail.com>

Update README.md file for PostgreSQL version 16 support (apache#1463)

Updated the README.md file for the master branch to reflect support
of PostgreSQL version 16.

    modified:   README.md

Clean up #included files in parser directory (apache#1465)

Cleaned up #included files in the .c files in the parser directory.
Additionally, fixed one that was at the backend root.

    modified:   src/backend/age.c
    modified:   src/backend/parser/cypher_analyze.c
    modified:   src/backend/parser/cypher_clause.c
    modified:   src/backend/parser/cypher_expr.c
    modified:   src/backend/parser/cypher_gram.y
    modified:   src/backend/parser/cypher_item.c
    modified:   src/backend/parser/cypher_keywords.c
    modified:   src/backend/parser/cypher_parse_node.c
    modified:   src/backend/parser/cypher_parser.c

Remove redundant job from CIs (apache#1473)

- Removed the job for setting the appropriate tag of docker image
  because it is not required anymore.

Add an additional way to find a previous variable ref (apache#1450)

Added an additional way to find a previous variable reference to
`transform_column_ref_for_indirection`.

There was no impact on current regression tests.

Add checks for array functions to recognize and decode VPC (apache#1064)

* Add checks for array functions to recognize and decode VPC

- Added for array functions size, head, last, isEmpty, reverse,
  agtype_in_operator and agtype_access_slice.

- Fixed a bug where object input to reverse would terminate the server
  instead of erroring out.

- Added regression tests.

* Fix issue with in operator tranformation

- We need to build a function call here if the rexpr is already
  tranformed. It can be already tranformed cypher_list as columnref.

Advance to Apache AGE version 1.5.0 (apache#1482)

Updated the following files to advance the Apache AGE version to
1.5.0

    modified:   Makefile
    modified:   README.md
    modified:   RELEASE
    modified:   age.control

Corrected whitespace issues in -

    modified:   sql/age_agtype.sql
    modified:   sql/agtype_coercions.sql
    modified:   sql/agtype_comparison.sql
    modified:   sql/agtype_gin.sql
    modified:   sql/agtype_operators.sql
    modified:   sql/agtype_string.sql

Correct cleanup of age--x.x.x.sql files (apache#1505)

Corrected the cleanup that I added to the Makefile for the
age--x.x.x.sql files.

    modified:   Makefile

This change is NOT needed for the other branches as it was added
as part of the release cleanup.

Clean up #included files in nodes, executor, & optimizer directories (apache#1509)

Cleaned up #included files in the .c files in the nodes, executor,
and optimizer directories.

    modified:   src/backend/executor/cypher_create.c
    modified:   src/backend/executor/cypher_delete.c
    modified:   src/backend/executor/cypher_merge.c
    modified:   src/backend/executor/cypher_set.c
    modified:   src/backend/executor/cypher_utils.c
    modified:   src/backend/nodes/ag_nodes.c
    modified:   src/backend/nodes/cypher_copyfuncs.c
    modified:   src/backend/nodes/cypher_outfuncs.c
    modified:   src/backend/optimizer/cypher_createplan.c
    modified:   src/backend/optimizer/cypher_pathnode.c
    modified:   src/backend/optimizer/cypher_paths.c

Clean up #included files in catalog & commands directories (apache#1514)

Cleaned up #included files in the .c files in the catalog and
commands directories.

    modified:   src/backend/catalog/ag_catalog.c
    modified:   src/backend/catalog/ag_graph.c
    modified:   src/backend/catalog/ag_label.c
    modified:   src/backend/commands/graph_commands.c
    modified:   src/backend/commands/label_commands.c

Bump gopkg.in/yaml.v3 in /drivers/golang (apache#1202)

Bumps gopkg.in/yaml.v3 from 3.0.0-20200313102051-9f266ea9e77c to 3.0.0.

---
updated-dependencies:
- dependency-name: gopkg.in/yaml.v3
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Update README.md

Clean up #included files in src/include directories (apache#1518)

Cleaned up #included files in the .h files in the src/include
directories.

Additionally, deleted 1 unnecessary header file.

    modified:   src/include/catalog/ag_catalog.h
    modified:   src/include/catalog/ag_graph.h
    modified:   src/include/catalog/ag_label.h
    modified:   src/include/catalog/ag_namespace.h
    modified:   src/include/commands/graph_commands.h
    modified:   src/include/commands/label_commands.h
    modified:   src/include/executor/cypher_executor.h
    modified:   src/include/executor/cypher_utils.h
    modified:   src/include/nodes/ag_nodes.h
    modified:   src/include/nodes/cypher_copyfuncs.h
    modified:   src/include/nodes/cypher_nodes.h
    modified:   src/include/nodes/cypher_outfuncs.h
    modified:   src/include/nodes/cypher_readfuncs.h
    modified:   src/include/optimizer/cypher_createplan.h
    modified:   src/include/optimizer/cypher_pathnode.h
    modified:   src/include/parser/cypher_clause.h
    modified:   src/include/parser/cypher_expr.h
    modified:   src/include/parser/cypher_gram.h
    modified:   src/include/parser/cypher_item.h
    modified:   src/include/parser/cypher_parse_node.h
    modified:   src/include/parser/cypher_parser.h
    modified:   src/include/parser/cypher_transform_entity.h
    modified:   src/include/utils/ag_cache.h
    modified:   src/include/utils/ag_float8_supp.h
    modified:   src/include/utils/ag_func.h
    modified:   src/include/utils/age_global_graph.h
    modified:   src/include/utils/agtype.h
    modified:   src/include/utils/agtype_parser.h
    modified:   src/include/utils/agtype_raw.h
    modified:   src/include/utils/graphid.h
    modified:   src/include/utils/load/ag_load_edges.h
    modified:   src/include/utils/load/ag_load_labels.h
    modified:   src/include/utils/load/age_load.h

    deleted:    src/include/utils/ag_load.h

Fix apache#1513 - Invalid variable reuse in CREATE and MERGE clause (apache#1515)

Allow the reuse of a previously declared vertex variable only if the succeeding
CREATE/MERGE vertex has associated edges
i.e CREATE (n) CREATE (n)             -- invalid
    CREATE (n) CREATE (n)-[:edge]->() -- valid

Fixed another invalid variable reuse where in a CREATE path, edge variable
could be duplicated which resulted in ambiguous reference error

Added regression tests

Update age_load to load scalar property values with appropriate type (apache#1519)

Previously, property values from csv files were always loaded as strings. This
patch converts a value to an appropriate scalar type (i.e. string, bool,
numeric, null) while loading. It uses the agtype_value_from_cstring()
function for conversion.

Additional change(s):
-------------------
 - Fix: for csv rows in edge files, create_agtype_from_list_i()'s start_index
   is corrected to 4

Add template for upgrading between versions of Apache AGE (apache#1506)

Added a template to be used when upgrading between versions of
Apache AGE.

This template is to be added to for all additions, deletions,
and modifications to the sql files that define operators, types,
and functions.

Modified .gitignore to allow these changes.

    modified:   .gitignore
    new file:   age--1.5.0--y.y.y.sql

Fix json serialization error in Python Driver (apache#1228)

* * Added test to catch the error in toJson()
* removed the trailing comma in _nodeToJson() node.properties part
* removed the trailing comma in _nodeToString() node.properties part

* * Added missing ", ".join() _nodeToString() in node.properties part

---------

Co-authored-by: Olivier <oliv.maurel@gmail.com>

Update the Go driver documentation, Linux installer, and CI (apache#1527)

Updated the Go driver documentation, Linux installer, and Github
CI file. The documentation needed to clarify what was needed. The
installer needed to be fixed. The CI file needed to have the
updated Go versions.

The driver installation for Linux will no longer install Java,
Golang, or ANTLR for the user. This is left to the users system
administrator. Instead, it will validate the current installation
of those components, then build and install the driver.

Also updated -
    modified:   drivers/golang/go.mod
    modified:   drivers/golang/go.sum

Update age_load to make property value conversion optional (apache#1525)

This is an addition to a previous patch that introduced loading
property values from CSV files as agtype. This behavior is
made optional in this patch with a boolean function parameter
`load_as_agtype`. When this parameter is false, values are
loaded as string.

Update README.md (apache#1526)

Amended AGE community at bottom of Readme file.
- Github issue and discussion link added
- Link to Discord was deleted.
- Added Stack overflow with questions and answers comment
- Changed Twitter as X

Fix unsorted output of some queries in the cypher_match test (apache#1507)

Fix unsorted output of some queries in the cypher_match test

Add optional parameter '=' in property constraints (apache#1516)

- The '=' operator checks if the original property value(as a whole) is
  equal to the given value.

  e.g MATCH (n {school:{addr:{city:'Toronto'}}}) tranforms into

      either(in case age.enable_containment is off)
      `properties.school.addr.city = 'Toronto'`

      or(in case age.enable_containment is on)
      `properties @> {school:{addr:{city:'Toronto'}}}`

      But MATCH (n ={school:{addr:{city:'Toronto'}}}) will tranform into

      either(in case age.enable_containment is off)
      `properties.school = {addr:{city:'Toronto'}}`

      or(in case age.enable_containment is on)
      `properties @>> {school:{addr:{city:'Toronto'}}}`

- Added @>> and <<@ operators. Unlike @> and <@, these operators does
  not recurse into sub-objects.

- Added regression tests.

- Added changes in sql files to version update template file.

Update Go installation and add in parser files (apache#1582)

Updated Go installation and added in built parser files.

While the parser files can be built by the user, there may
be instances where the user doesn't have the ability to do so.
For the latter case, they have been added.

    modified:   drivers/golang/go.mod
    modified:   drivers/golang/go.sum

    new file:   drivers/golang/parser/age_base_listener.go
    new file:   drivers/golang/parser/age_base_visitor.go
    new file:   drivers/golang/parser/age_lexer.go
    new file:   drivers/golang/parser/age_listener.go
    new file:   drivers/golang/parser/age_parser.go
    new file:   drivers/golang/parser/age_visitor.go

Implement list comprehension (apache#1610)

* Add initial code for list comprehension

- Additionally, fixed a bug in nested queries.

* Resolve variable scope and ambigous variable issue

- For scoping the variable, remove the pnsi from namespace list after
  being used by the list comprehension.

- To resolve the ambigous variable issue, only scan the pnsi of
  list_comp subquery.

* Fix non agg plan node issue in list comprehension

- Resolved the issue 'aggref found in non-agg plan node'
  when the list comprehension is used as a property constraint in
  MATCH and other clauses or when used in WHERE clause. Intead of adding
  qual in the jointree, we are now adding the qual in having node of
  query.

  e.g.
  MATCH (n {list: [for i in [1,2,3]]})
  MATCH (n) WHERE n.list=[for i in [1,2,3]]
  WITH n WHERE n.list=[for i in [1,2,3]]

- Removed redundant call to function tranform_cypher_clause_with_where
  by tranform_match_clause.

* Resolve variable reuse issue in list comprehension

- Also, for scoping list comprehension variable, instead of removing
  the pnsi of list comp from namespace, now we just mark the cols of
  pnsi as not visible.

* Modify grammer to match opencypher list comprehension grammer

- Also added initial regression tests for list comprehension

* Add regression tests for bugs resolved

- Includes tests for nested cases, ambigous variable issue, list
  comprehension variable scoping and planner node aggref issue.

- Should be added more.

* Resolve invalid output on multiple list comprehensions

- Fix the output of multiple list comprehensions in the RETURN clause
  and the WHERE clause.
- Added regression tests for the above and some for the previous bugs
  resolved.

* Fix aggref found where not expected issue (apache#189)

- Resolved the 'aggref found where not expected' error thrown when
  using list comprehensions in UNWIND, or as a property update
  expression in SET or in the comparison expressions, typecasts or
  string matching in WITH - WHERE clause

- Added an error to be thrown when using aggregation functions in UNWIND
  and property update expr in SET clause

- Added regression tests

* cleanup: Remove dead code and formatting changes

---------

Co-authored-by: John Gemignani <jrgemignani@gmail.com>
Co-authored-by: Zainab Saad <105385638+Zainab-Saad@users.noreply.github.com>

Fix Issue 1630: MERGE using array not working in some cases (apache#1636)

This PR fixes the issue where some previous clause user variables
were not getting passed to the next clause.

Basically, there is a function, remove_unused_subquery_outputs,
that tries to remove unnecessary variables by replacing them, in
place, with a NULL Const. As these variables are needed, we need
to wrap them with the volatile wrapper to stop that function from
removing them.

Added regression tests.

Fix Issue 1634: Setting all properties with map object causes error (apache#1637)

Fixed the issue where setting the properties would not work, if it where
an object passed as the properties.

Added regression tests.

Implement EXISTS Subquery      (apache#1562)

* Add grammar rules for exists subquery

Add grammar rule for exists subquery. EXISTS subquery can be used
with UNION and does not have to feature a RETURN.

Regression tests not yet added.

* Implement EXISTS Subquery

Implements a naive version of EXISTS subquery that evaluates the
underlying subquery.

Add regression tests related to EXISTS

TODO: Implement logic to allow the underlying subquery to constrain
the queries in the outer scope.

Also fixes a minor typo.

Allow agtype_build_map to use AGTYPE keys (apache#1666)

Added the ability for agtype_build_map to use AGTYPE keys.

In doing this, age_tostring needed to be broken up and rewritten
to handle UNKNOWNOID. This rewrite allows other functions to use
the logic in age_tostring without using age_tostring directly.

Additionally, rewrote age_tostring as a ("any") type function; it
used to be a (variadic "any"). The logic here can be used to re-
write other (variadic "any") functions that have a set number of
input parameters.

Added regression tests.

Sample code for AGE-JDBC driver (apache#390)

* sample code for age-jdbc-drivers

* cleaned the code

* update query statement

Add hooks for multi-arch builds on dockerhub (apache#1683)

- hooks/build overrides the default build command to use buildx and
  create an image for amd64 and arm64 archictectures. The arm64 build
  will take time to build as it is emulated using qemu.
- hooks/push overrides the default push command to push the multi-arch
  images. The push is initiated from the buildx context since we have
  the push argument(--push) set in the buildx command.

Implement map projection (apache#1710)

Adds support for openCypher Map Projection specification.

Update README.md (apache#1713)

Remove Discord Badge in README.md

Fix Issue 1691 - MERGE incorrectly creates multiple vertices (apache#1718)

Fixed issue 1691 where MERGE would incorrectly create multiple
vertices. This only occurred when MERGE was being driven by a
previous clause.

NOTE: To be more correct, the issue is with creating duplicate
      paths.

The reason this happened was due to the visibility of tuples that
were created during the MERGE instance. It is not possible to add
them in to be rescanned.

Because of this limitation, it required adding the ability to
MERGE to know what path an instance had already created.

Added regression tests.

Fix shift/reduce conflict in grammar (apache#1719)

- The grammar had a shift/reduce conflict due to the ambiguity of the
  `IN` keyword. This is resolved by adding generic rule and manually
  resolving to the correct specific rule.
- Added additional test cases.

Fix Issue 1709 - MERGE creates incomplete vertices after the first one (apache#1721)

Fixed issue 1709 where MERGE appears to create incomplete vertices after
the first one. However, the real issue was that SET wasn't seeing the
newly created tuples from MERGE. This was due to an incorrect cid when
performing the tuple insert in MERGE.

The issue was that the cid was being overwritten in the function
insert_entity_tuple_cid. Once this was corrected, everything worked fine.

Added regression tests.

Update README.md (apache#1720)

1) Simplified the Introductory Section

2) Deleted the [Community] section

Remove duplicate check (apache#1740)

This commit removes [a benign] redundant check for label validity

Added Networkx Support in python driver (apache#1716)

Co-authored-by: munmud <moontasir042@gmail.com>

Update README.md (apache#1728)

- Review the Quick Start section in order to make it more concise and less redundant.
- The commands weren't working if issued in the written order.
- I kept only the commands that create entities with labels and properties, rather than separate commands for each step.

Add helpful messages to the VLE subsystem (apache#1742)

Added helpful messaging to the VLE subsystem.

These changes are due to issue 365 where there was an issue with
the VLE that turned out to not be with the VLE. Basically, there
is a way, apparently, to have a malformed edge or possibly one not
fully removed. When the VLE subsystem does its load on the graph,
it would error out on these edges as this was considered to be a
potential data corruption issue.

The changes added are to report and ignore these edges. Reporting
them, so as to allow the user to take corrective action by removing
them. Ignoring them, as they are likely not relevant to the query.
Basically, leaving it up to the user to decide.

No impact to current regression tests.

Add the command graph_stats and improve VLE messaging for load (apache#1750)

Added the command graph_stats. This command will force a reload of
the specified graph. This is done to generate current statistics
and dump any warning messages about the graph generated during a
the load.

Cleaned up some messaging from the VLE load process for duplicate
vertices and edges.

Added regression tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants