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

GH-2864: Fix algebra function name for the hash functions. #2865

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

afs
Copy link
Member

@afs afs commented Nov 23, 2024

GitHub issue resolved #2864

The algebra operator symbol should be lower case.
Print in SPARQL is overridden to be upper case as it has been for a long time.

Some reformatting in-passing.


  • Tests are included
  • Key commit messages start with the issue number (GH-xxxx)

By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.

@afs afs changed the title GH-2864: GH-2864: Fix algebra function name for the hash functions. Nov 23, 2024
Copy link
Member

@rvesse rvesse left a comment

Choose a reason for hiding this comment

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

Change looks good

Do we want some test cases that cover the scenario that prompted the bug report?

Given your analysis that this has existed basically forever in the code base that strongly implies there are no pre-existing test cases that expose this bug

@afs
Copy link
Member Author

afs commented Nov 25, 2024

It's a display bug in printing algebra. The execution of queries isn't affected which presumably is why is hasn't come to light.

With the exception of qparse, I can't think of any non-development case of writing algebra derived from query syntax in a way that is going to be read in again. Writing is mainly for development, and parsing algebra for testing isn't based on the output of query syntax. Some of this "grew" in stages over an extended (pre-ASF) period of time.

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

Successfully merging this pull request may close these issues.

is this valid sparql or does qparse have a bug?
2 participants