Skip to content

fix(sql): fix rewrite sample by to group by same column names' bug. #5113

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

Merged
merged 9 commits into from
Dec 20, 2024

Conversation

kafka1991
Copy link
Collaborator

@kafka1991 kafka1991 commented Oct 29, 2024

Fixes #5105
Fixes #5164

@kafka1991 kafka1991 changed the title fix(sql): rewrite sample by to group by same columns bug fix. fix(sql): rewrite sample by to group by same column names bug fix. Oct 29, 2024
@kafka1991 kafka1991 changed the title fix(sql): rewrite sample by to group by same column names bug fix. fix(sql): rewrite sample by to group by same column names bug. Oct 29, 2024
@kafka1991 kafka1991 changed the title fix(sql): rewrite sample by to group by same column names bug. fix(sql): rewrite sample by to group by same column names' bug. Oct 29, 2024
@kafka1991 kafka1991 changed the title fix(sql): rewrite sample by to group by same column names' bug. fix(sql): fix rewrite sample by to group by same column names' bug. Oct 29, 2024
@nwoolmer nwoolmer added Bug Incorrect or unexpected behavior SQL Issues or changes relating to SQL execution labels Oct 29, 2024
@nwoolmer nwoolmer self-requested a review November 15, 2024 12:23
@nwoolmer
Copy link
Contributor

Hey @kafka1991 , do you have bandwidth to get this up to date with master? If not, I can pick this up this week.

Sorry for the delays gettin this wrapped up!

Also, here is a test case:

select  dateadd('d', 1, timestamp) "timestamp" , symbol from trades
sample by 10m

@kafka1991
Copy link
Collaborator Author

kafka1991 commented Nov 25, 2024

Hey @kafka1991 , do you have bandwidth to get this up to date with master? If not, I can pick this up this week.

Sorry for the delays gettin this wrapped up!

@nwoolmer Have already rebase master onto current branch. Please continue to take a review.

Also, here is a test case:

select  dateadd('d', 1, timestamp) "timestamp" , symbol from trades
sample by 10m

This test can also be passed in the current branch.

@ideoma ideoma self-requested a review December 4, 2024 15:58
@kafka1991 kafka1991 requested a review from ideoma December 6, 2024 17:10
@bluestreak01
Copy link
Member

another issue in the same area, the following SQL fails with obscure error:

SELECT timestamp+60000000 as 'timestamp', timestamp, 0 AS "extra_column"
FROM sample_table
WHERE name = 'XYZ'
SAMPLE BY (1m);

@kafka1991
Copy link
Collaborator Author

kafka1991 commented Dec 17, 2024

another issue in the same area, the following SQL fails with obscure error:

SELECT timestamp+60000000 as 'timestamp', timestamp, 0 AS "extra_column"
FROM sample_table
WHERE name = 'XYZ'
SAMPLE BY (1m);

This issue and #5164 are both in the same area and have been fixed in the PR.

@bluestreak01
Copy link
Member

epic, thank you!

@bluestreak01 bluestreak01 merged commit c821b34 into questdb:master Dec 20, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Incorrect or unexpected behavior SQL Issues or changes relating to SQL execution
Projects
None yet
4 participants