Skip to content

Add an ARRAY<string> column to one_row_complex through its definition only #877

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
laughingman7743 wants to merge 1 commit into from
Closed

Add an ARRAY<string> column to one_row_complex through its definition only #877

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions tests/pyathena/tables.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ def _to_text(value: Any) -> str:
pa.struct([("a", pa.int32()), ("b", pa.int32())]),
),
Column("col_decimal", "DECIMAL(10,1)", pa.decimal128(10, 1)),
Column("col_array_string", "ARRAY<string>", pa.list_(pa.string())),

@laughingman7743 laughingman7743 Sep 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Self-review round one (implementation behavior): CLEAN

Base 3202ab5 (#876 head, the stacked base), head bcbf874. The full diff is 2 lines.

  • Generation: Table.data_file writes ["a", "b"] as list<string> and the new round-trip check accepts it. The DDL gains col_array_string ARRAY<string>.
  • Derived tests: all 19 whole-row tests pick the column up through Selection. The PolarsCursor tests leave it out through COMPLEX_FAMILIES.
  • Other readers of the table:
    • The tests that select explicit one_row_complex columns are unaffected.
    • meta.reflect() over the schema (test_get_table_names/test_get_view_names) reflects the new array type.
    • The Glue-vs-Athena and throttled-metadata comparisons list the table with the new column on both sides.
  • AWS results: 104 passed for the derived-expectation, reflection, and one_row_complex column tests, and 43 passed for the Glue, throttling, and table-metadata tests.

@laughingman7743 laughingman7743 Sep 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent review (relayed Codex result): CLEAN

Reviewer: Codex CLI 0.157.1 (codex exec -s read-only), model gpt-6-astra, session 01a0e7b6-1295-7331-87c3-e21bcaa43fd8.

Scope: base 3202ab5, head bcbf874. The review ran on a detached snapshot without .env, and the prompt left out the PR framing. The snapshot was still clean at the head afterwards. This was a static review only.

Covered, as reported:

  • DDL, Parquet generation, and the Arrow round-trip guard.
  • All consumers under tests/, including metadata, Glue, reflection, and schema listings, plus assertions that depend on column count or position.
  • The derived rows, descriptions, pandas dtypes, Arrow schemas, Polars dtypes, and SQLAlchemy array element types.
  • The default, S3FS, pandas, Arrow, and Polars converter and result-set paths, including UNLOAD.

Result: "No broken or newly vacuous assertions found. The new value's expectations match the applicable cursor paths, and ["a", "b"] is losslessly representable by pa.list_(pa.string())."

),
rows=(
(
Expand All @@ -211,6 +212,7 @@ def _to_text(value: Any) -> str:
[(1, 2), (3, 4)],
{"a": 1, "b": 2},
Decimal("0.1"),
["a", "b"],

@laughingman7743 laughingman7743 Sep 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Self-review round two (claims, callers, AWS operations): CLEAN

Base 3202ab5, head bcbf874.

Claims checked against the AWS run on this head

  • Each row of the description's table is a derived expectation that passed on AWS:
    • ["a", "b"] for the Python-object cursors and SQLAlchemy
    • "[a, b]" for the CSV paths
    • pa.list_(pa.field("array_element", pa.string())) / pl.List(pl.String) for Arrow UNLOAD
    • object dtype for pandas UNLOAD
  • "Two lines, both in the table definition": the diff touches only tests/pyathena/tables.py.
  • "Four places before": the gzipped Hive text row, the Jinja DDL, the hand-written expectations in about 20 tests, and len(one_row_complex.c) == 16. All four existed before Define the shared test tables in Python and generate their data聽#874.

Operational effect: none beyond one more column in one Parquet file. The number of setup statements and uploads is unchanged.

CI scope: no SQLAlchemy or Spark path changes, so on Ready the PyAthena suite runs without Spark.

),
),
)
Expand Down
Loading