Define the shared test tables in Python and generate their data - #874
Merged
Merged
Conversation
The session fixtures were a Jinja DDL template plus hand-made data files, including a gzipped Hive text row with \002/\003 separators for the complex columns. tests/pyathena/tables.py now defines each table once: columns with their Athena and Arrow types, rows as Python values, storage, comment, and table properties. The session setup generates the DDL and the data files from it and uploads them; the Spark tests' CSV file is generated as well. Tables are Parquet except one_row, whose SerDe and delimiters the reflection tests assert, and many_rows, which tests read without ORDER BY: Athena returned a Parquet file of 10,000 rows out of file order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
laughingman7743
commented
Sep 28, 2026
| name: str | ||
| columns: tuple[Column, ...] | ||
| rows: tuple[tuple[Any, ...], ...] = () | ||
| storage: Literal["parquet", "text"] = "parquet" |
Member
Author
There was a problem hiding this comment.
Self-review round one (implementation behavior): FINDINGS (1 simplification, fixed)
Base 7b4cceb (#873 head, the stacked base), head 12cfd65. Full diff reviewed (10 files).
- Finding:
Table.storagewas a plainstr. A misspelled value would silently produce Parquet DDL and data. It is nowLiteral["parquet", "text"], in 2760077. No behavior change;just lintandmypy tests/pyathena/tables.pypass. - DDL:
- Text tables emit the old
ROW FORMAT DELIMITED ... STORED AS TEXTFILEclause verbatim, so the one_row reflection asserts (SerDe, input/output format,field.delim,line.delim,serialization.format) still hold. parquet_with_compressionkeepsSTORED AS PARQUETandTBLPROPERTIES ('parquet.compress'='SNAPPY').CREATE VIEWreplacesCREATE OR REPLACE VIEW, which is safe because every session uses a fresh schema.
- Text tables emit the old
- Data:
- Parquet is written with an explicit schema from
Column.arrow_type. The live Athena results match the old files: see the parity check in the PR description. timestamp("ms"),map_from key/value tuples,decimal128(10,1)andbinaryall read back as before, andtest_complexpasses for every cursor.- NULLs in
integer_na_values/boolean_na_valuesare Parquet nulls, and the NA tests pass.
- Parquet is written with an explicit schema from
- Row order:
many_rowsstays text after the observed Parquet reordering. The 3-row NA tables are single-page Parquet files, and theirSELECT *order tests pass. - Resources:
_data_objectsis cached per process (ENV.schemais fixed at import), so setup uploads and teardown deletes the same keys. When the upload fails midway,pytest_sessionfinishis skipped, as before this change, and the 1-day lifecycle rule removes the objects. - Python 3.10:
str | Noneannotations andzip(strict=True)are 3.10-compatible.pa.Table.from_pylistexists in the pyarrow>=10 floor.
| storage="text", | ||
| comment="table comment", | ||
| ), | ||
| # A text table: tests read it without ORDER BY and expect the file order, |
Member
Author
There was a problem hiding this comment.
Self-review round two (claims, callers, AWS operations): FINDINGS (description only, fixed)
Claims checked:
- "Same statements as before": after Give the executemany and partition tests their own tables #873 the template had 7 tables and 2 views, and
TABLES/VIEWShave the same 7 and 2. - "7 objects per process": before, 6 row files plus
test.dat; now 5 data files (partition_tableandparquet_with_compressionhave no rows) plus the Spark CSV plustest.dat. - "Byte-identical": checked offline for the
one_row,many_rowsandspark_group_by.csvbytes. - "Out of file order": seen in the first targeted run, where
test_pandas_cursor_chunked_vs_regular_same_datagot0..6, 15..30, 7... After the switch to text, the order-sensitive-kset (371 passed) includes both failed tests. - Removed-file references:
git grepover the repository finds no remaining reference totests/resources/rows,create_table.sqlor the row file names. The license-header config entry is removed, and NOTICE and docs never listed them. - CI path filters: nothing under
pyathena/(aio/)?sqlalchemy,tests/sqlalchemy,tests/pyathena/(aio/)?sqlalchemyor the Spark paths changed, so CI runs only the PyAthena suite, without Spark tests.
Finding: the TEST section attributed the Spark test_spark_sql skips to xdist without evidence. The skip reason printed by -rs is pytest-dependency's depends on test_spark_dataframe. The description now quotes that reason and notes the Literal-only commit after the tested commit.
laughingman7743
commented
Sep 28, 2026
|
|
||
| def _upload_rows(): | ||
| @functools.cache | ||
| def _data_objects(): |
Member
Author
There was a problem hiding this comment.
Independent review (relayed Codex result): CLEAN
- Reviewer: Codex CLI 0.157.1 (
codex exec -s read-only), model gpt-6-astra, session 01a0e72e-c8d6-7830-88e2-bb7c702d0fd3. - Scope: base 7b4cceb, head 2760077. The review ran on a detached snapshot worktree without
.env. The prompt left out the PR number, description, commit messages, and self-review findings. After the review, the snapshot was unchanged and still at the reviewed head. - Covered surfaces, as reported:
- the generated DDL and data compared with all removed resources, including the decompressed Hive row;
- types, values, NULLs, row order, reflection metadata, views, and the consumers of the Spark CSV;
- S3 keys, upload/delete symmetry, worker isolation, and setup/cleanup failure paths;
- Python and PyArrow compatibility, workflow dependencies, and remaining references to removed files.
- Result: "No correctness regressions found by static inspection." This was a static review only: no builds, tests, or network access.
laughingman7743
marked this pull request as ready for review
September 28, 2026 10:09
This was referenced Sep 28, 2026
This was referenced Sep 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WHAT
tests/pyathena/tables.pydefines the shared test tables and views once, in Python:Column(name, athena_type, arrow_type, comment)Table(name, columns, rows, storage, partitions, comment, tblproperties), whererowsare Python values in column order andstorageis"parquet"or"text"View(name, query)TABLESandVIEWShold the same tables and views thatcreate_table.sql.jinja2created:one_row,many_rows,one_row_complex,partition_table,integer_na_values,boolean_na_values,parquet_with_compression,view_one_row,v_one_row.Table.create_statementgenerates the DDL, andTable.data_filegenerates the data file: Parquet written with pyarrow using the column's Arrow type, or tab-separated text.spark_group_by.csvis generated fromSPARK_GROUP_BY.tests/pyathena/conftest.py: the session setup uploads the generated files (_upload_data,_delete_data) and creates the tables and views from the definitions (_create_tables). The data files go to<schema>/<table>/data.parquetordata.tsv. The Spark CSV and the filesystem test file keep their keys.tests/resources/rows/(6 files) andtests/resources/queries/create_table.sql.jinja2, together with its entry inscripts/config/license_headers.toml.Storage:
one_rowLazySimpleSerDe, text input format, andfield.delim/line.delimSerDe propertiesmany_rowsORDER BYand expect file order. With a Parquet file,SELECT * FROM many_rows LIMIT 100returned rows out of order (0..6, 15..30, 7..), which failedtest_pandas_cursor_chunked_vs_regular_same_dataandtest_pandas_cursor_iter_chunks_consistencyone_row_complex,integer_na_values,boolean_na_valuesone_row_complex)partition_table,parquet_with_compressionpartition_tablechanges from text to Parquet; the tests only read its metadataWHY
Part of #848 (step 1), for #834. Stacked on #873.
one_row_complex.gzwas a single gzipped Hive text row with\002/\003separators, which could not practically be edited by hand. Adding a column meant editing that file and the DDL template separately. Each table is now one entry, and the DDL and data are generated from it. Deriving the test expectations from these definitions is the next step of #848.The session runs the same statements as before: 7
CREATE EXTERNAL TABLEand 2CREATE VIEWper test process. It uploads the same number of objects, 7 per process: 5 data files, the Spark CSV, and the filesystem test file.TEST
Tested commit: 12cfd65. The only later commit types
Table.storageasLiteral["parquet", "text"], with no change in behavior;just lintpassed on it.just lintpassed.one_rowandmany_rowsdata andspark_group_by.csvare byte-identical.SELECT *(all rows, in order) andDESCRIBEfor all 7 tables and 2 views, and found no differences. This ran whilemany_rowswas still Parquet, and even so its 10,000 rows came back in file order that time.many_rowsis now text again, byte-identical to the old file.pytest -n 4 tests/pyathena -k "complex or na_values or reflect or table_options or get_columns or table_comment or view or glue or as_pandas or partition or spark_dataframe or spark_sql or table_names or has_table or chunk": 282 passed, 2 skipped. The 2 failures were themany_rowsordering failures above, which led to keeping it as text.pytest -n 4 tests/pyathena -k "many or fetchmany or fetchall or iter or chunk or arraysize": 371 passed, 1 skipped, including the two tests that had failed.pytest -n 1 -rs tests/pyathena -k "spark_dataframe or spark_sql": 4 passed, 2 skipped. All threetest_spark_dataframetests read the generatedspark_group_by.csvand passed. Twotest_spark_sqltests were skipped by pytest-dependency (depends on test_spark_dataframe); the third passed.changesjob skips the SQLAlchemy compliance suites and the Spark tests in CI.🤖 Generated with Claude Code