Conversation
Texera read CSV, JSONL, Arrow and plain text off disk but not Parquet, the format most tables in a data-science workflow are already stored in. Converting one to CSV first loses what the file knew: the column written as an INTEGER came back as text for the schema to guess at again. The file states its own types in a footer, so this source infers nothing. It reads a row group at a time rather than the whole file, which is the thing a columnar format is chosen to avoid, and a column that is a group, a list or a map is refused by name instead of being dropped in silence. A timestamp is read with UTC arithmetic, matching what ArrowUtils means by a Texera TIMESTAMP: the count from the epoch lands on a wall clock, and no zone of the machine's own enters it. Reading it any other way would move the value and part the engine from the script the export writes. No new dependency: parquet-hadoop and parquet-column are already on the classpath under iceberg-parquet, and the reader takes a LocalInputFile so none of Hadoop's own file plumbing is involved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
feature
frontend
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8512 +/- ##
============================================
- Coverage 93.62% 92.74% -0.88%
- Complexity 4857 4927 +70
============================================
Files 1212 1239 +27
Lines 50037 52266 +2229
Branches 6132 6446 +314
============================================
+ Hits 46847 48476 +1629
- Misses 1676 2196 +520
- Partials 1514 1594 +80
*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:
|
📊 Arrow Flight E2E bench(no arrow-flight-e2e.csv in artifact) |
|
@carloea2 a new source, still a draft like the other two new operators: it reads a Parquet file, taking the column types from the file's own footer rather than inferring them. Would you take a look when you have a moment? |
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
Two data-loss cases reproduced with real Parquet files and the compiled native reader.
A DECIMAL column was read as the integer it is stored in, so a file meaning 12.34 arrived as 1234. It is read as the number the scale makes of it, in each of the three storages Parquet allows for one, and the exported script casts the column pandas fills with Decimal objects so both sides hold the same type. A timestamp counted in microseconds or nanoseconds was rounded to the millisecond on the way into a java.sql.Timestamp, which holds nanos. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
I retested the decimal and timestamp fixes. The Parquet source looks good.
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
Repeated primitive Parquet fields are accepted as scalar columns. The native reader keeps only occurrence zero, while pandas keeps the repeated values. Please reject repeated fields or read them the same way on both paths, and add a repeated field test.
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
Unsigned Parquet integers are also read differently. The native path ignores the unsigned annotation, so a maximum unsigned 32 bit value becomes minus one while pandas reads 4294967295. Please handle unsigned annotations consistently and add boundary tests.
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
More Parquet types differ. INT96 becomes bytes natively but timestamps in pandas. TIME and DURATION stay integers natively but pandas converts them. Also offset plus limit can overflow and create the wrong slice. Please normalize or reject these cases and add boundary files.
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
Parquet float columns stay as float32 in the exported path, while native widens them to double. With stored values 16777216 and 1 followed by Sum, native returns 16777217 but the export returns 16777216. Please widen Parquet float fields to double and add a downstream numeric test.
A column the file states one thing about and pandas another was read as its storage, so the engine and the exported script answered differently about the same bytes. A repeated column kept only its first value where pandas hands back the whole list. An unsigned column was read signed, so the largest unsigned 32-bit value arrived as -1 rather than 4294967295. An INT96 arrived as twelve raw bytes rather than the timestamp they are, and a JSON column as bytes rather than the text it holds. Each of those is now read as the value the file means, and the ones with no Texera column to be are refused by name as a nested column already is: a repeated column, a time of day, an interval, a 16-bit float, and an unsigned 64-bit integer, which is wider than any Texera column. The script side keeps a Parquet FLOAT in single precision, so a column holding 16777216 and 1 summed to 16777216 where the executor, widening to double, gets 16777217. It also reads the writer's own Arrow types back out of the file's metadata, which the footer does not state: a duration returns as a timedelta and a timestamp written in a zone returns in that zone, where the executor reads the count and the UTC wall clock. The generated code puts all three back to what the operator reads. Finally, the end of the scan window is counted in Long. Two Ints the panel accepts add up past what an Int holds, and the slice would come out negative and take the wrong rows. The spec now runs the exported script under python over a file holding these columns and compares it to the executor cell by cell, so the parity is run rather than asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
All four are real, and all four are fixed.
The spec now runs the exported script under python and compares it to the executor cell by cell. |
…rt names it The source export replaced `sourceBasename` with `standaloneSourcePath` and a placeholder the translator resolves, so a generator that spells the file name itself both calls a method that is going away and gives two sources reading different files whose paths end alike the same name. Offer the path and let the translator name it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
Nullable long values still lose precision in the exported Parquet source. I wrote a Parquet column containing 9007199254740993 and null, then read it with the generated pandas path. It returns 9007199254740992 because the column becomes float64. Native reads the exact long. Please preserve nullable integers before pandas converts them and add this case.
A nullable long lost precision in the exported script: pandas widens a holed integer column through a float, where every value past 2^53 is rounded, so 9007199254740993 came back as ...992. The executor reads the exact long off the same file. A holed 32-bit integer went the same way, its column arriving as a float where the engine keeps INTEGER. The read asks for the nullable dtypes, as the Arrow source already does, and the normalization beside it is named in the nullable spelling and widens into the nullable dtypes in turn, so a hole stays a hole rather than the NaN a numpy column would have to write it as. The parity test grows a row with a hole in every column, which is what costs a numpy column its type. That test could not have caught this before: it never bound the `sourceFile` its own body names, so the script stopped there and the comparison never ran. It binds it now, and its driver reads pd.NA and NaT as the null the executor hands over. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Fixed in A holed 32-bit integer went the same way, and the parity test here had not been running at all: it never bound the |
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
Reproduced locally with the generated Python code. One correctness issue below. I did not run a frontend workflow or the full Scala suite.
| // value past 2^53 is rounded and 9007199254740993 came back as ...992. The | ||
| // executor reads the exact long, and the declared column stays integral. | ||
| val read = | ||
| s"""out1df = pd.read_parquet($SourceFilePlaceholder, dtype_backend="numpy_nullable")""" |
There was a problem hiding this comment.
A Parquet file written with a named pandas index loses that column in the export. I tested customer_id values 101 and 102: the footer contains customer_id, so the native reader exposes it, but pandas restores it as the index and a downstream projection raises KeyError. Please preserve every physical column and add a named-index test.
There was a problem hiding this comment.
There was a problem hiding this comment.
Retested fcd65b6. The named customer_id column is preserved with values 101 and 102. This finding is fixed.
A file pandas wrote from a frame keyed by one of its columns records that in the footer, and pandas reads those columns back as the frame's index rather than as columns. The executor reads the columns the footer states and has no notion of an index, so a file written from a frame keyed by `customer_id` kept that column on the one side and dropped it on the other, where a projection naming it raised. The footer is handed back to the reader stripped of its metadata, so pandas reads every column the file holds, under the name the footer gives it and in the order it states them. Restoring the index afterwards would not do: an unnamed one comes back as `index` where the file calls it `__index_level_0__`, and it lands first where the footer puts it last. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 participants
What changes were proposed in this PR?
A source that reads a Parquet file. Texera read CSV, JSONL, Arrow and plain text off disk but not Parquet, and converting one to CSV first loses what the file knew: the column written as an INTEGER came back as text for the schema to guess at again.
The format states its own types in a footer, so this source infers nothing. It reads a row group at a time rather than the whole file, which is the thing a columnar format is chosen to avoid. A column Texera has no value for is refused by name instead of being dropped in silence: a group, a list, a map, a repeated column, a time of day, an interval, a 16-bit float, or an unsigned integer wider than a Texera one.
A timestamp is read with UTC arithmetic, matching what
ArrowUtilsmeans by a Texera TIMESTAMP: the count from the epoch lands on a wall clock, and no zone of the machine's own enters it. Reading it any other way would move the value and part the engine from the script the export writes.No new dependency.
parquet-hadoopandparquet-columnare already on the classpath undericeberg-parquet, and the reader takes aLocalInputFile, so none of Hadoop's own file plumbing is involved.Any related issues, documentation, discussions?
Not part of #8325: nothing here makes a workflow exportable. It does ship standalone code, so it reads the trait that issue introduced.
It stays a draft until the set in #8325 has merged, together with the two other new operators, rather than because anything here is unfinished: the trait it reads landed with #8502, and this branch builds and tests green on
maintoday.Closes #8511, the task this change is the whole of.
How was this PR tested?
Twenty-two tests in the operator's own spec, over Parquet files the spec writes: the columns the footer states, the values read back, a row that wrote no field at all, the timestamp that a zone could have moved, an unsigned column at its largest value, a timestamp written in the twelve bytes an older writer used, the columns refused by name, a file that is not Parquet, and the Python the export emits with each scan window.
The last of them runs that Python. It writes a file holding every column the two sides could read differently, runs the exported script, and compares what it reads to what the executor reads, cell by cell.
The parity harness needs no per-operator code for it. A scan source is fixtured by the format it declares, so an encoder for
"Parquet"is the whole of what it asks for, and that entry lands with the harness in #8364. With both in place the operator runs on the canonical table both ways and the two answers match.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
🤖 Generated with Claude Code