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

[TTAHUB-1828] Change all "GROUP BY TRUE" to "GROUP BY 1=1" to support postgres 15.x #1723

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

Conversation

GarrettEHill
Copy link
Collaborator

@GarrettEHill GarrettEHill commented Sep 19, 2023

Description of change

Postgres 15 does not like "GROUP BY TRUE"
https://postgrespro.com/list/thread-id/2661353
work around is to use 1=1 in place of true

Include change to move postgres from 12.4 to 15.4

How to test

tests work, can create a report

Issue(s)

Checklists

Every PR

  • Meets issue criteria
  • JIRA ticket status updated
  • Code is meaningfully tested
  • Meets accessibility standards (WCAG 2.1 Levels A, AA)
  • API Documentation updated
  • Boundary diagram updated
  • Logical Data Model updated
  • Architectural Decision Records written for major infrastructure decisions
  • UI review complete

Production Deploy

  • Staging smoke test completed

After merge/deploy

  • Update JIRA ticket status

Copy link
Collaborator

@AdamAdHocTeam AdamAdHocTeam left a comment

Choose a reason for hiding this comment

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

I get a weird error starting docker with this branch:

image

Let me know if I just need to tweak my config.

@GarrettEHill
Copy link
Collaborator Author

@kcirtapfromspace can you look at this

@kcirtapfromspace
Copy link
Collaborator

@GarrettEHill How do you feel about this comment in that thread?

Sure, you can write any constant expression, for instance NULL::bool
would work. The question is where do we draw the line between SQL92
and SQL99 behaviors. I think "an undecorated constant is SQL92, and
therefore it must be an integer matching an output column number" is
a nice simple rule. The fact that TRUE and FALSE were not previously
treated as undecorated constants is an unintentional wart of the old
implementation, not something we ought to preserve.

I guess the conversation in there leans towards not maintaining support for group by true this going forward. I'm curious if 1=1 is an obscure format that isn't really covered in the documentation. What alternatives might be used?

@thewatermethod thewatermethod removed their request for review January 11, 2024 16:27
@thewatermethod
Copy link
Collaborator

@GarrettEHill did you say yesterday Angela had found an issue with testing this?

@GarrettEHill
Copy link
Collaborator Author

@GarrettEHill did you say yesterday Angela had found an issue with testing this?

The issue was not with this, but a bug on resource dashboard

Copy link
Collaborator

@thewatermethod thewatermethod left a comment

Choose a reason for hiding this comment

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

Tested locally, and on dev. Runs on CI.

@AdamAdHocTeam AdamAdHocTeam removed their request for review May 9, 2024 16:14
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.

None yet

4 participants