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: Add GroupColumn for Date/Time/Timestamp #13457

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jonathanc-n
Copy link
Contributor

Which issue does this PR close?

Closes #13263.

Rationale for this change

What changes are included in this PR?

Add group column for Date/Time/Timestamp

Are these changes tested?

Are there any user-facing changes?

@jonathanc-n jonathanc-n marked this pull request as draft November 17, 2024 23:35
@github-actions github-actions bot added the physical-expr Physical Expressions label Nov 17, 2024
@jonathanc-n
Copy link
Contributor Author

jonathanc-n commented Nov 17, 2024

Comparing main and column
--------------------
Benchmark clickbench_1.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃       main ┃     column ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0     │     0.27ms │     0.36ms │  1.34x slower │
│ QQuery 1     │    61.02ms │    36.84ms │ +1.66x faster │
│ QQuery 2     │    84.63ms │    83.85ms │     no change │
│ QQuery 3     │    78.49ms │    70.84ms │ +1.11x faster │
│ QQuery 4     │   696.13ms │   695.54ms │     no change │
│ QQuery 5     │   765.24ms │   806.28ms │  1.05x slower │
│ QQuery 6     │    33.10ms │    31.55ms │     no change │
│ QQuery 7     │    40.46ms │    41.95ms │     no change │
│ QQuery 8     │   797.77ms │   720.54ms │ +1.11x faster │
│ QQuery 9     │  1057.00ms │   993.54ms │ +1.06x faster │
│ QQuery 10    │   248.37ms │   246.95ms │     no change │
│ QQuery 11    │   261.37ms │   282.06ms │  1.08x slower │
│ QQuery 12    │   869.76ms │   926.57ms │  1.07x slower │
│ QQuery 13    │  1338.20ms │  1382.54ms │     no change │
│ QQuery 14    │   819.75ms │   802.72ms │     no change │
│ QQuery 15    │   856.39ms │   873.74ms │     no change │
│ QQuery 16    │  1892.26ms │  1881.67ms │     no change │
│ QQuery 17    │  1798.52ms │  1794.75ms │     no change │
│ QQuery 18    │  4686.36ms │  4779.32ms │     no change │
│ QQuery 19    │    61.05ms │    66.55ms │  1.09x slower │
│ QQuery 20    │  1132.85ms │  1268.51ms │  1.12x slower │
│ QQuery 21    │  1512.94ms │  1720.92ms │  1.14x slower │
│ QQuery 22    │  4829.86ms │  4858.37ms │     no change │
│ QQuery 23    │ 11642.12ms │ 10491.15ms │ +1.11x faster │
│ QQuery 24    │   633.42ms │   570.52ms │ +1.11x faster │
│ QQuery 25    │   511.58ms │   508.50ms │     no change │
│ QQuery 26    │   586.58ms │   689.01ms │  1.17x slower │
│ QQuery 27    │  1842.41ms │  1985.72ms │  1.08x slower │
│ QQuery 28    │ 14969.19ms │ 14861.69ms │     no change │
│ QQuery 29    │   540.19ms │   525.39ms │     no change │
│ QQuery 30    │   838.59ms │   828.83ms │     no change │
│ QQuery 31    │   787.30ms │   842.87ms │  1.07x slower │
│ QQuery 32    │  4365.25ms │  3927.03ms │ +1.11x faster │
│ QQuery 33    │  9277.87ms │  6284.31ms │ +1.48x faster │
│ QQuery 34    │  7823.31ms │  6524.80ms │ +1.20x faster │
│ QQuery 35    │  1121.27ms │  1112.43ms │     no change │
│ QQuery 36    │   130.37ms │   125.53ms │     no change │
│ QQuery 37    │    90.24ms │    90.44ms │     no change │
│ QQuery 38    │    91.87ms │    92.78ms │     no change │
│ QQuery 39    │   235.79ms │   228.69ms │     no change │
│ QQuery 40    │    32.08ms │    29.45ms │ +1.09x faster │
│ QQuery 41    │    29.82ms │    29.51ms │     no change │
│ QQuery 42    │    36.08ms │    36.22ms │     no change │
└──────────────┴────────────┴────────────┴───────────────┘

Giving weird results, trying to fix. Is someone else able to run this on their computer and see what they get.

@jonathanc-n
Copy link
Contributor Author

@alamb Was this the sort of implementation you were looking for? (will add tests)

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

Successfully merging this pull request may close these issues.

Implement Specialized GroupColumn for Date/Time/Timestamp types for multi-column GROUP BY
1 participant