Conversation
Automated Reviewer SuggestionsBased on the
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 482 | 0.294 | 18,610/37,704/37,704 us | 🔴 +12.6% / 🔴 +143.9% |
| 🔴 | bs=100 sw=10 sl=64 | 1,122 | 0.685 | 86,748/137,853/137,853 us | 🔴 +24.2% / 🔴 +30.2% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,392 | 0.85 | 712,951/796,667/796,667 us | ⚪ within ±5% / 🟢 +31.9% |
Baseline details
Latest main 75c85aa from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 482 tuples/sec | 542 tuples/sec | 796.29 tuples/sec | -11.1% | -39.5% |
| bs=10 sw=10 sl=64 | MB/s | 0.294 MB/s | 0.331 MB/s | 0.486 MB/s | -11.2% | -39.5% |
| bs=10 sw=10 sl=64 | p50 | 18,610 us | 16,533 us | 12,549 us | +12.6% | +48.3% |
| bs=10 sw=10 sl=64 | p95 | 37,704 us | 34,084 us | 15,458 us | +10.6% | +143.9% |
| bs=10 sw=10 sl=64 | p99 | 37,704 us | 34,084 us | 19,275 us | +10.6% | +95.6% |
| bs=100 sw=10 sl=64 | throughput | 1,122 tuples/sec | 1,206 tuples/sec | 1,027 tuples/sec | -7.0% | +9.3% |
| bs=100 sw=10 sl=64 | MB/s | 0.685 MB/s | 0.736 MB/s | 0.627 MB/s | -6.9% | +9.3% |
| bs=100 sw=10 sl=64 | p50 | 86,748 us | 80,265 us | 100,085 us | +8.1% | -13.3% |
| bs=100 sw=10 sl=64 | p95 | 137,853 us | 110,960 us | 105,895 us | +24.2% | +30.2% |
| bs=100 sw=10 sl=64 | p99 | 137,853 us | 110,960 us | 116,076 us | +24.2% | +18.8% |
| bs=1000 sw=10 sl=64 | throughput | 1,392 tuples/sec | 1,392 tuples/sec | 1,055 tuples/sec | 0.0% | +31.9% |
| bs=1000 sw=10 sl=64 | MB/s | 0.85 MB/s | 0.85 MB/s | 0.644 MB/s | 0.0% | +31.9% |
| bs=1000 sw=10 sl=64 | p50 | 712,951 us | 714,986 us | 979,292 us | -0.3% | -27.2% |
| bs=1000 sw=10 sl=64 | p95 | 796,667 us | 786,266 us | 1,018,835 us | +1.3% | -21.8% |
| bs=1000 sw=10 sl=64 | p99 | 796,667 us | 786,266 us | 1,039,692 us | +1.3% | -23.4% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,414.98,200,128000,482,0.294,18609.51,37703.84,37703.84
1,100,10,64,20,1781.95,2000,1280000,1122,0.685,86748.48,137852.64,137852.64
2,1000,10,64,20,14367.65,20000,12800000,1392,0.850,712951.04,796666.74,796666.74Aggregate groups by a column's values, and a timestamp has a different value in almost every row, so grouping by one puts each row in its own group. Sales by month could not be asked for, though the platform carries a TIMESTAMP type, a Gantt chart and a time-series plot. Nothing here is parsed: the column is already a moment by the time it arrives, and which text became which moment was settled upstream, the way KNIME, Alteryx and Spark all separate parsing from extraction. The parts are read as ISO-8601 states them, since the two runtimes do not agree by default — pandas counts Monday as 0 and java.time counts it as 1. Each part is added under a name derived from the column it came from, so reading two timestamp columns names four distinct results. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
d6f6a34 to
535f06e
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8480 +/- ##
============================================
- Coverage 93.62% 92.77% -0.86%
- Complexity 4857 4925 +68
============================================
Files 1212 1239 +27
Lines 50037 52186 +2149
Branches 6132 6423 +291
============================================
+ Hits 46847 48413 +1566
- Misses 1676 2191 +515
- Partials 1514 1582 +68
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@carloea2 this adds an operator that reads the year, month, weekday and the rest out of a timestamp column, each as a whole-number column of its own. Would you take a look when you have a moment? |
carloea2
left a comment
There was a problem hiding this comment.
The timestamp fields match between the native and exported paths. Looks good.
carloea2
left a comment
There was a problem hiding this comment.
The standalone path cannot handle timestamps outside pandas nanosecond range. A timestamp in year 2500 works in the native Java path, but pd.to_datetime raises before extracting the year. Please preserve the supported timestamp range or reject it consistently, and add boundary tests.
pandas parses into nanoseconds by default, which reach only 1677 to 2262, so the exported script ended the run on a moment the executor reads like any other: it holds a java.sql.Timestamp, where the year 2500 is ordinary. Microseconds cover the whole range the executor answers for, and the fields are compared against it at and past the nanosecond edge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Fixed in c4bb488: the exported script now parses the column at microsecond resolution, which covers the whole range the executor answers for, so the year 2500 no longer ends the run. The boundary test reads the ten fields at 1677-09-22, 2262-04-11, 2500-01-01, 1500-06-15 and 9999-12-31, and compares each one against the executor's own answer for the same moment. |
carloea2
left a comment
There was a problem hiding this comment.
The timestamp range issue is fixed. I checked the wider dates and null handling with pandas 2.2.3. The added test also compares the fields against the native executor. Looks good.
What changes were proposed in this PR?
An operator that reads the year, quarter, month, day, weekday, day of year, ISO week, hour, minute or second out of a timestamp column, each added as a whole-number column of its own.
Nothing here is parsed. The column is already a moment when it arrives, and which text became which moment was settled upstream, the way KNIME, Alteryx and Spark all separate parsing from extraction. The parts are read as ISO-8601 states them, because the two runtimes disagree by default: pandas counts Monday as 0,
java.timecounts it as 1.Any related issues, documentation, discussions?
Not part of #8325: that set makes the operators Texera already had exportable. This adds a new one, which implements the trait from the start rather than gaining it. The trait is in #8327, which has merged.
Closes #8478, the task this change is the whole of.
Aggregate groups by a column's values, and a timestamp has a different value in almost every row, so grouping by one puts each row in its own group. Sales by month could not be asked for, though the platform carries a TIMESTAMP type, a Gantt chart and a time-series plot.
How was this PR tested?
Twelve unit tests in the operator's own spec, over the schema it declares, the values the executor reads and the Python it emits. The parity harness runs it on 13 configurations, one per field plus a hostile column name and an empty cell, comparing the engine's answer against the exported script's.
The operator in the editor, reading
event_timeand asking for the year, the month and the weekday:The three whole-number columns beside the timestamp they came from. 2024-01-15 is a Monday, 2024-07-04 a Thursday and 2025-02-09 a Sunday, reading 1, 4 and 7 as ISO-8601 states them rather than the 0, 3 and 6 pandas would give:
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)