Nightly upstream merge - #83
Merged
Merged
Conversation
Cached name mappings only accumulate spellings, so after a case-colliding table or dataset was dropped or renamed, lookups kept failing with an ambiguity error for up to bigquery.case-insensitive-name-matching.cache-ttl.
PartitionedOutputOperator.finish() flushes the partitioner before returning it to the pool. When the flush failed, for example because the exchange sink of an aborted fault-tolerant task was already removed, the partitioner was neither returned to the pool nor closed, so its memory reservation was never released and leaked from the worker memory pool.
docker-images 132 upgrades the spark4-iceberg test image to Spark 4.1.0, which requires bumping the iceberg-spark-runtime artifact used in product tests from the 4.0 to the 4.1 classifier (iceberg-spark-runtime-4.1_2.13) to avoid classpath skew between the Spark engine and the Iceberg runtime jar (e.g. NoSuchMethodError on Catalyst's Origin constructor). Also updates the expected Spark exception for the known ORC nested partition column limitation (apache/iceberg#3139), whose exact message now varies by product test environment.
Basic stage stats reported progress only once every registered stage was scheduled, and computed it as completed drivers over total drivers across all stages. Fault tolerant execution creates and schedules stages lazily, so a single stage waiting for its inputs hid the progress of the whole query, and stages not created yet were ignored. Now progress is reported as soon as any stage is scheduled, every stage weighs the same, and plan stages not created yet count as 0%. This matches how QueryStateMachine computes progress for fault tolerant queries. If more stages are registered than the current plan has fragments, the registered count is used. Pipelined execution keeps the previous behavior.
at_timezone changes only the zone a timestamp with time zone value is rendered in, never the instant, and timestamp with time zone comparisons are instant-based. A comparison over at_timezone(column, zone) therefore constrains the underlying column to exactly the range a direct comparison would, but DomainTranslator previously gave up on it, losing partition pruning and other connector pushdown for such predicates - notably views that expose at_timezone(ts, zone_column) over partitioned Iceberg tables. Extract the instant range as a TupleDomain on the underlying column while keeping the original expression as the remaining predicate, so the zone argument's null and invalid-zone behavior is preserved for rows that are actually read. Exercise the DomainTranslator at_timezone domain derivation on Iceberg: comparisons over at_timezone(ts, zone) on a month(ts)-partitioned table prune to the matching partition's split whether the zone argument is a constant or a column, and results match a pushdown-disabled run. Pin the pruning boundary with an invalid-zone row: a derivable instant range prunes the row before evaluation and the query succeeds, while a non-deterministic disjunct prevents domain derivation, the row is evaluated, and the invalid zone fails the query. The row is inserted after the verifySplitCount checks because their pushdown-disabled cross-run would evaluate the invalid zone on every row. Cover a DST-ambiguous wall time (2020-10-25 02:31:18 Europe/Warsaw occurs at two instants): wall-time resolution happens during analysis, before domain extraction, so pruning must match exactly the rows the comparison itself matches. Skip the split-count tests under fault-tolerant execution, where verifySplitCount fails because operator summaries are not collected reliably, the same way as testSplitPruningForFilterOnPartitionColumn.
createQueryRunner intermittently fails with a ContainerFetchException when pulling quay.io/keycloak/keycloak, since KeycloakContainer only gets a single retry by default. Bump the container startup retry limit so the whole container start, including image resolution, gets a couple of extra attempts to absorb that flake.
bill-ph
approved these changes
Sep 26, 2026
bill-ph
left a comment
bill-ph
left a comment
There was a problem hiding this comment.
Reviewed the PR description, diff, and discussion, then checked the surrounding planner, scheduler, exchange, metadata, and BigQuery paths. No P0 or P1 issues found.
P2
- BigQuery's refreshed ambiguity handling performs a remote lookup on every subsequent access while a case-insensitive name collision persists. The dataset path invalidates and relists datasets, and the table path reruns
INFORMATION_SCHEMA.TABLES. A short refresh interval would retain recovery from stale collisions without making persistent collisions repeatedly call BigQuery.
Overengineering and scope creep check: the merge's changes appear tied to their respective upstream fixes; no separate scope issue found. Tests, builds, and linters were not run as requested.
— Robo Bill v2 (gpt-6-sol, high reasoning)
| } | ||
| // The colliding table may have been dropped or renamed, so invalidate the entry and re-resolve. | ||
| // The rebuild below seeds from the cache, so the entry must be removed before it runs. | ||
| remoteTableCaseInsensitiveCache.invalidate(cacheKey); |
There was a problem hiding this comment.
P2: While a table-name collision persists, every lookup invalidates this cache entry and runs the INFORMATION_SCHEMA.TABLES query again. The dataset ambiguity path similarly invalidates and relists all datasets on every access. Persistent ambiguous names can therefore turn repeated failed lookups into repeated remote requests despite the configured cache TTL; consider bounding how often an ambiguous entry is refreshed.
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
11 participants
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.
Automated nightly merge of
trinodb/trinomaster into the PostHog fork.\n\nLocal sanity build:JAVA_HOME=/usr/lib/jvm/jdk-25.0.4.1+1 ./mvnw install -pl plugin/trino-ducklake -am -DskipTests -Dair.check.skip-all=true✅