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

Negative indices on arrays seem to return nothing/null #240

Closed
selendym opened this issue Jul 11, 2022 · 12 comments
Closed

Negative indices on arrays seem to return nothing/null #240

selendym opened this issue Jul 11, 2022 · 12 comments
Labels
bug Something isn't working

Comments

@selendym
Copy link

Describe the bug
Negative indices on arrays seem to return nothing/null.

How are you accessing AGE (Command line, driver, etc.)?
With psql from the docker image. One container for the server and another one for the client.

What data setup do we need to do?

select * from ag_catalog.create_graph('g');

What is the necessary configuration info needed?
Just the docker image built from master branch.

What is the command that caused the error?

select * from ag_catalog.cypher('g', $$ with [0, 1, 2, 3] as l return l[-1] $$) as (r0 ag_catalog.agtype);

The above returns (note the empty line):

 r0 
----
 
(1 row)

Compare with:

select * from ag_catalog.cypher('g', $$ with [0, 1, 2, 3] as l return l[3] $$) as (r0 ag_catalog.agtype);
select * from ag_catalog.cypher('g', $$ with [0, 1, 2, 3] as l return l[-1..] $$) as (r0 ag_catalog.agtype);

which return:

 r0 
----
 3
(1 row)

and:

 r0  
-----
 [3]
(1 row)

Expected behavior
The negative index should work as described in: https://age.apache.org/docs/master/intro/types.html#negative-index-access

Environment (please complete the following information):

  • env: docker
  • branch: master
  • commit: 6fbdfa8
@selendym selendym added the bug Something isn't working label Jul 11, 2022
@jrgemignani
Copy link
Contributor

I will take a look

@jrgemignani
Copy link
Contributor

I'm not sure how this happened. There will be a patch to fix this shortly.

jrgemignani added a commit that referenced this issue Jul 11, 2022
Fix github issue #240 - negative array bounds.

The issue was that the wrong variable name was used for the array
index. It needed to be array_index, but was index instead. The reason
this was not caught was -

     1) There weren't any regression tests for this functionality.
     2) The name 'index' was defined elsewhere as either a function
        or a struct. So, the compiler was okay with the particular
        usage.

The variable name was changed to the correct name.

Regression tests were added.
@jrgemignani
Copy link
Contributor

This has been corrected, and is available, in the master branch -

psql-11.5-5432-pgsql=# select * from ag_catalog.cypher('g', $$ with [0, 1, 2, 3] as l return l[-1] $$) as (r0 ag_catalog.agtype);
 r0
----
 3
(1 row)

@selendym
Copy link
Author

Thanks for the quick fix!

@selendym
Copy link
Author

selendym commented Jul 12, 2022

Not sure if this is related, but seems very similar:

select * from ag_catalog.cypher('g', $$ with {k0: [0, 1], k1: [2, 3]} as m return m.k0[1] $$) as (r0 ag_catalog.agtype);
--  r0 
-- ----
--  1
-- (1 row)

select * from ag_catalog.cypher('g', $$ with {k0: [0, 1], k1: [2, 3]} as m return m.k0[-1] $$) as (r0 ag_catalog.agtype);
--  r0 
-- ----
--  
-- (1 row)

select * from ag_catalog.cypher('g', $$ with {k0: [0, 1], k1: [2, 3]} as m return m.k0[-1..] $$) as (r0 ag_catalog.agtype);
-- ERROR:  slice must access a list

Note the second part, where the result is missing, and the third, which results in an error (while it likely should not).

@selendym selendym reopened this Jul 12, 2022
@selendym
Copy link
Author

Nested arrays seem to have similar problems:

select * from ag_catalog.cypher('g', $$ with [[0, 1], [2, 3]] as l return l[0][1] $$) as (r0 ag_catalog.agtype);
--  r0 
-- ----
--  1
-- (1 row)

select * from ag_catalog.cypher('g', $$ with [[0, 1], [2, 3]] as l return l[0][-1] $$) as (r0 ag_catalog.agtype);
--  r0 
-- ----
--  
-- (1 row)

select * from ag_catalog.cypher('g', $$ with [[0, 1], [2, 3]] as l return l[0][-1..] $$) as (r0 ag_catalog.agtype);
--     r0    
-- ----------
--  [[0, 1]]
-- (1 row)

Note the second part, where the result is missing, and the third, which doesn't seem to make sense.

@jrgemignani
Copy link
Contributor

Yeah, it is in the same access operator functions. It is getting the wrong size of the returned array, since it is in a binary format.

@jrgemignani
Copy link
Contributor

The other issue, with the slice operator, needs to be fixed as well. It was do to some inadvertent code changes.

@jrgemignani
Copy link
Contributor

There is a pending patch to correct this. It should be applied within the next few days.

@selendym
Copy link
Author

Thanks. Feel free to close this ticket when that gets merged.

muhammadshoaib pushed a commit to muhammadshoaib/age that referenced this issue Jul 13, 2022
Fix github issue apache#240 - negative array bounds.

The issue was that the wrong variable name was used for the array
index. It needed to be array_index, but was index instead. The reason
this was not caught was -

     1) There weren't any regression tests for this functionality.
     2) The name 'index' was defined elsewhere as either a function
        or a struct. So, the compiler was okay with the particular
        usage.

The variable name was changed to the correct name.

Regression tests were added.
muhammadshoaib added a commit to muhammadshoaib/age that referenced this issue Jul 13, 2022
jrgemignani added a commit that referenced this issue Jul 13, 2022
Fixed github issue #240 - negative array bounds - addendum.

There was found to be an issue with the transform_A_Indirection
function. Previous work on the function removed a few important
lines. This caused it to improperly transform nested indirections.
These lines were added back in.

Additionally, since the previous work on transform_A_Indirection,
some new functionality was added to transform_ColumnRef. This
allowed the removal of some unnecessary code as well.

Added regression tests to cover nested access and slice operations.
@jrgemignani
Copy link
Contributor

The patch was applied and this should be corrected.

If this fixes the problem for you, go ahead and close the ticket. It is preferred that those who open tickets close them when the issue is resolved.

@selendym
Copy link
Author

Yes, this fixes the issue. Thanks again.

muhammadshoaib pushed a commit to muhammadshoaib/age that referenced this issue Jul 13, 2022
Fixed github issue apache#240 - negative array bounds - addendum.

There was found to be an issue with the transform_A_Indirection
function. Previous work on the function removed a few important
lines. This caused it to improperly transform nested indirections.
These lines were added back in.

Additionally, since the previous work on transform_A_Indirection,
some new functionality was added to transform_ColumnRef. This
allowed the removal of some unnecessary code as well.

Added regression tests to cover nested access and slice operations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants