Skip to content

Conversation

@ggevay
Copy link
Contributor

@ggevay ggevay commented Jan 15, 2026

The first commit removes the old timestamp selection, as discussed here: https://materializeinc.slack.com/archives/C08A62E0751/p1767794273757089?thread_ts=1764865885.225539&cid=C08A62E0751

The second commit updates a bunch of tests and fixes the error message when we don't find a suitable timestamp: Both the old and the new timestamp selection were relying on generate_timestamp_not_valid_error_msg, but that doesn't work well for the new timestamp selection for AS OF queries: the candidate comes out not as the AS OF time, so

SELECT * FROM t AS OF 1

would regress from

Timestamp (1) is not valid for all inputs: [u1]

to

Timestamp (1768481949666) is not valid for all inputs: []

which does not make sense: The [] is the list of inputs that this timestamp is not valid for, so we can see that actually this timestamp is valid for all inputs. But the problem here is not that this timestamp is not valid for an input, but the problem is the AS OF, which is not mentioned by the error msg.

So, the PR makes the error message print the constraints instead. It also tweaks the formatting of constraints a bit, to use more user-facing terminology, e.g., instead of ComputeInput we now say Indexed input. So, now we have stuff like:

select * from t AS OF 7;

ERROR:  could not find a valid timestamp for the query
DETAIL:  Constraints:
lower:
  (Storage inputs: [u1]): [1768492534743 (2026-01-15 15:55:34.743)]
upper:
  (Query's AS OF): [            7 (1970-01-01 00:00:00.007)]

Edit: I'm still fighting with a lot of annoying test output rewrites.

@ggevay ggevay added the A-ADAPTER Topics related to the ADAPTER layer label Jan 15, 2026
@ggevay ggevay force-pushed the remove-old-timestamp-selection branch 12 times, most recently from ece50ad to e5b7d27 Compare January 15, 2026 15:53
@ggevay ggevay marked this pull request as ready for review January 15, 2026 15:56
@ggevay ggevay requested review from a team as code owners January 15, 2026 15:56
@ggevay ggevay requested a review from aljoscha January 15, 2026 15:56
@ggevay ggevay force-pushed the remove-old-timestamp-selection branch 2 times, most recently from 685f53f to cf35b02 Compare January 15, 2026 16:36
@ggevay ggevay force-pushed the remove-old-timestamp-selection branch 2 times, most recently from f39cf63 to 1cc5929 Compare January 16, 2026 16:10
Copy link
Contributor

@def- def- left a comment

Choose a reason for hiding this comment

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

I like removing code!

# -----
# Others (ordered by name)
"allow_real_time_recency": "true",
"constraint_based_timestamp_selection": "verify",
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to keep this for older versions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I've added it back.

@ggevay ggevay force-pushed the remove-old-timestamp-selection branch 4 times, most recently from 97c59bd to 319a8be Compare January 16, 2026 19:24
@ggevay ggevay force-pushed the remove-old-timestamp-selection branch from 319a8be to 4ddb963 Compare January 18, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ADAPTER Topics related to the ADAPTER layer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants