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 945 - incorrect count(*) return values (#1288) #1312

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

jrgemignani
Copy link
Contributor

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.

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.
@github-actions github-actions bot added the PG14 PostgreSQL14 label Oct 26, 2023
@MuhammadTahaNaveed MuhammadTahaNaveed merged commit 162bf30 into apache:PG14 Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PG14 PostgreSQL14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants