-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: make the arguments print themselves with type info #16232
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16232 +/- ##
==========================================
+ Coverage 68.60% 68.61% +0.01%
==========================================
Files 1544 1544
Lines 198016 198065 +49
==========================================
+ Hits 135849 135903 +54
+ Misses 62167 62162 -5 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Dirkjan Bussink <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
@dbussink would you mind reviewing this PR? |
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
---------------------------------------------------------------------- | ||
---------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clearly some diff I cannot see.
case *SubQuery: | ||
return pushOrderingToOuterOfSubquery(ctx, in, src) | ||
} | ||
return in, NoRewrite | ||
} | ||
|
||
func pushOrderingToOuterOfSubquery(ctx *plancontext.PlanningContext, in *Ordering, sq *SubQuery) (Operator, *ApplyResult) { | ||
outerTableID := TableID(sq.Outer) | ||
for idx, order := range in.Order { | ||
deps := ctx.SemTable.RecursiveDeps(order.Inner.Expr) | ||
if !deps.IsSolvedBy(outerTableID) { | ||
return in, NoRewrite | ||
} | ||
in.Order[idx].SimplifiedExpr = sq.rewriteColNameToArgument(order.SimplifiedExpr) | ||
in.Order[idx].Inner.Expr = sq.rewriteColNameToArgument(order.Inner.Expr) | ||
} | ||
sq.Outer, in.Source = in, sq.Outer | ||
return sq, Rewrote("push ordering into outer side of subquery") | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this an unreachable code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was not used in any of the queries we tested. this is handled pretty early in the planning process, when the subqueries are still inside a SubqueryContainer, so removing this code made no difference
Signed-off-by: Andres Taylor <[email protected]>
Description
Currently, arguments are sent as a map to the vttablet, which then incorporates them into the query string using literals. This method works well for types with robust literal support but poses issues for types that do not, such as dates and times.
This PR addresses the issue by altering the handling of types that cannot be easily represented as literals. For these types, we will now use strings and the CAST function to convert them into the correct type within the query, ensuring the values are correctly interpreted by the database.
Example:
The query that triggered this work was:
This would be split up into two different queries:
If the first query returns a decimal value of 50, the second query becomes:
This converts the value to a bigint, not a decimal, which is unexpected.
Related Issue(s)
Fixes #16261
Checklist