dolthub/doltgresql#2373 fix VALUES subquery unwrap from dropping clauses#2402
Merged
Hydrocharged merged 3 commits intodolthub:mainfrom Mar 4, 2026
Conversation
nodeAliasedTableExpr unconditionally unwrapped any subquery over a VALUES clause when the inner SELECT had a single FROM item. This discarded projections, LIMIT, ORDER BY, DISTINCT, WHERE, GROUP BY, and HAVING because the guard only checked len(inSelect.From) == 1. Replace that check with isTrivialSelectStar, which only permits unwrapping when the inner SELECT is a bare SELECT * with no additional clauses. Non-trivial subqueries now correctly fall through to the vitess.Subquery path, preserving all semantics. Additionally, add a *plan.Offset guard to the TypeSanitizer so OFFSET literals inside subqueries are kept as GMS types, matching the existing *plan.Limit guard. Without this, OFFSET triggered "invalid type: bigint" because GMS validateOffsetAndLimit only recognizes its own type singletons. Tests: - Un-skip three mixed-type VALUES-in-subquery assertions (projection, LIMIT, ORDER BY) - Add mixed-type (int/decimal) variants to all integer-only subquery test groups (DISTINCT, WHERE, GROUP BY, HAVING, column aliasing, combined clauses, JOIN) - Add OFFSET-only and LIMIT+OFFSET subquery assertions for both integer and mixed-type VALUES Refs: dolthub#2373
codeaucafe
force-pushed
the
codeaucafe/2373-fix-subquery-values-projection-aggregate-distinct
branch
from
078265c to
d7b3469
Compare
Contributor
Author
|
@Hydrocharged sorry to ping ya, but since this kinda corresponds to my last PR you assisted me on maybe you could review this or at least determine if this "fix" is on the right path by handling in analyzer and ast. Thanks in advance. |
Hydrocharged
reviewed
Mar 2, 2026
Inline isSelectStarOnly into isTrivialSelectStar. And, revert TypeSanitizer comment to match the original, and instead updated to mention both limit and offset literals. Refs: dolthub#2373
codeaucafe
added a commit
to codeaucafe/codeaucafe
that referenced
this pull request
Mar 4, 2026
Add latest Doltgres PR dolthub/doltgresql#2402
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
3 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.
Summary
len(inSelect.From) == 1guard innodeAliasedTableExprwithisTrivialSelectStar, which only unwraps bareSELECT * FROM (VALUES ...)subqueries with no additional clauses (projections, LIMIT, ORDER BY, DISTINCT, WHERE, GROUP BY, HAVING)
*plan.Offsetguard to TypeSanitizer to prevent OFFSET literals inside subqueries from being converted to Doltgres types, which caused GMSvalidateOffsetAndLimitto reject them with "invalid type: bigint"
Closes: #2373