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

feat(sql): implementation of 'distinct' algorithm for time series data #1053

Merged
merged 3 commits into from
May 24, 2021

Conversation

bluestreak01
Copy link
Member

No description provided.

@ideoma
Copy link
Collaborator

ideoma commented May 24, 2021

[PR Coverage check]

😍 pass : 18 / 18 (100.00%)

file detail

path covered line new line coverage
🔵 io/questdb/griffin/engine/groupby/DistinctTimeSeriesRecordCursorFactory.java 16 16 100.00%
🔵 io/questdb/griffin/SqlCodeGenerator.java 2 2 100.00%

compiler,
sqlExecutionContext,
"y",
"select distinct * from x",
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about
select distinct * from x order by ts DESC

Copy link
Member Author

Choose a reason for hiding this comment

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

this should work too, in this scenario order by ts desc will be discarded (table is already ordered) and distinct will work as expected, but i will add a test


@Override
public void recordAt(Record record, long atRowId) {
baseCursor.recordAt(record, atRowId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is correct, records from this cursor don't map 1 to 1 to base cursor

Copy link
Member Author

Choose a reason for hiding this comment

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

this is tested to be correct :)

@bluestreak01 bluestreak01 merged commit 2764af9 into master May 24, 2021
@bluestreak01 bluestreak01 deleted the distinct_ts branch May 24, 2021 12:00
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

2 participants