Skip to content

Commit

Permalink
Merge #28039 #28040
Browse files Browse the repository at this point in the history
28039: sql: unblacklist repeat builtin r=jordanlewis a=jordanlewis

We have memory accounting for it now. Also, change a test that was
checking whether a top-k sort was correctly planned to a planner test
that uses EXPLAIN, instead of using the repeat builtin in a roundabout
fashion.

Release note (sql change): permit distribution of queries that use the
repeat builtin function.

28040: sql: mark DTime as having an ambiguous format r=jordanlewis a=jordanlewis

Previously, DTime wasn't correctly marked as an AmbiguousFormat type.
This caused misformatting of time types in DistSQL.

Release note (bug fix): fix formatting of time datatypes in some
circumstances.

Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
  • Loading branch information
craig[bot] and jordanlewis committed Jul 30, 2018
3 parents a1f35d3 + 1b1f18d + 6f46fa6 commit 3b4a31c
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 10 deletions.
7 changes: 0 additions & 7 deletions pkg/sql/logictest/testdata/logic_test/distsql_numtables
Original file line number Diff line number Diff line change
Expand Up @@ -197,13 +197,6 @@ SELECT y FROM (SELECT y FROM NumToStr ORDER BY y LIMIT 5) AS a WHERE y <> 2
4
5

statement error memory budget exceeded
SELECT y, str, repeat('test', y) AS res FROM NumToStr ORDER BY res

# Verify we use the "top K" strategy and thus don't hit OOM as above.
statement ok
SELECT y, str, repeat('test', y) AS res FROM NumToStr ORDER BY res LIMIT 10

# Regression test for #20481.
query I
SELECT count(*) FROM (SELECT 1 AS one FROM NumToSquare WHERE x > 10 ORDER BY xsquared LIMIT 10)
Expand Down
47 changes: 47 additions & 0 deletions pkg/sql/logictest/testdata/planner_test/distsql_sort
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# LogicTest: 5node-dist

statement ok
CREATE TABLE data (a INT, b INT, c INT, d INT, PRIMARY KEY (a, b, c, d))

# Split into ten parts.
statement ok
ALTER TABLE data SPLIT AT SELECT i FROM generate_series(1, 9) AS g(i)

# Relocate the ten parts to the five nodes.
statement ok
ALTER TABLE data EXPERIMENTAL_RELOCATE
SELECT ARRAY[i%5+1], i FROM generate_series(0, 9) AS g(i)

# Verify data placement.
query TTITI colnames
SHOW EXPERIMENTAL_RANGES FROM TABLE data
----
start_key end_key range_id replicas lease_holder
NULL /1 1 {1} 1
/1 /2 2 {2} 2
/2 /3 3 {3} 3
/3 /4 4 {4} 4
/4 /5 5 {5} 5
/5 /6 6 {1} 1
/6 /7 7 {2} 2
/7 /8 8 {3} 3
/8 /9 9 {4} 4
/9 NULL 10 {5} 5

# Ensure sort-all strategy is used
query BT
EXPLAIN(DISTSQL) SELECT * FROM data ORDER BY d
----
true https://cockroachdb.github.io/distsqlplan/decode.html#eJyskz1vwjAQhvf-jHetK-IkfGViZaEV7VZlcOMTigRxZBupFeK_V_mQaBA4Fu5oO-89fi6-EyolaSMOZJB9goMhBkMChhQMU-QMtVYFGaN080kXWMtvZBFDWdVH22znDIXShOwEW9o9IcOH-NrTloQkPYnAIMmKct9ial0ehP5ZSWEF8jODOtpLIWPFjpDxM_OHvSttSU-mQ84qfb5bPr5b_lJVaUma5K2iN-6wUS-qnvCh6z18MsBz_1by4FaOwPpWzh5tZezvEge7jMB6l_mjLom_SxLsMgLrXRaPuqT-Lmmwywisd1n-x7jeKL8lU6vKkNckRs0sk9xRN_tGHXVBb1oVLaZbvra5dkOSsd0p7xbrqjtqLvg3zJ3heBDm1-HYTR5BJ8506g6nIfeeOsMzN3kWQp47wws3eRFCXrr_VTTyTNyP7Jqdn59-AwAA__-GYYnr

# Ensure top-k strategy is used
query BT
EXPLAIN(DISTSQL) SELECT * FROM data ORDER BY d limit 10
----
true https://cockroachdb.github.io/distsqlplan/decode.html#eJyslE1r4zAQhu_7K5b3ulpi2c6XT7kGSlrS3ooPqjUEQ2IZSYGW4P9e_AGp00Q2yEdJeeeZZyLrgkJJ2okTGSTv4GAIwRCBIQbDHClDqVVGxihd_6QNbOUnkoAhL8qzrbdThkxpQnKBze2RkOBNfBxpT0KSngVgkGRFfmwwpc5PQn9tpLACacWgzvZayFhxICS8YuNhr0pb0rN5n7OJ_4HhKT_l9i8PHpLCh6QrQGlJmuTv-ml1p52d-q_KGb_RHmwk6jXCx8-Xe893ANbNdzHBfMPxWqG31gCs01pOoBWN14q8tQZgndZqAq14vFbsrTUA67TWE3_kd0h7MqUqDPVIjyoH9QtA8kDti2HUWWf0olXWYNrlc5NrNiQZ257ydrEt2qO6wZ9h7gyHvTC_DYdu8gA6cqZjdzj26XvuDC_c5IUPeekMr9zklQ957f6vgoFr4r5kt-y0-vMdAAD__55em18=

# Ensure chunk strategy is used
query BT
EXPLAIN(DISTSQL) SELECT * FROM data ORDER BY a, c
----
true https://cockroachdb.github.io/distsqlplan/decode.html#eJy0lLtugzAUhvc-RfSvcRUM5MaUNUtapd0qBhcfpUgEI9uRWkW8e8VFSokSg0Q62ubnO9_x5YxcSdqJIxlEH-Bg8MEQgCEEwxwxQ6FVQsYoXX3SBLbyG5HHkObFyVbTMUOiNCE6w6Y2I0R4F58Z7UlI0jMPDJKsSLMaU-j0KPTPRgorEJcM6mQvPzJWHAgRL9lw2JvSlvRs3uVs-JRtgikYjsImX5OM8mjC7xL9u8QLSGlJmuRtTlzeKG2nnlUx490W3Csh6JTAh3eYj-5wD6zt8OKBHfaH6_mj9Xpgrd7ygXrBcL1gtF4PrNVbPVAvHK4XjtbrgbV663-6_jeIezKFyg0NutVe9S6QPFDzlhh10gm9apXUmGb4UufqCUnGNqu8GWzzZqkq8G-YO8N-J8yvw76b3IMOnOnQHQ7H1D13hhdu8mIMeekMr9zk1Rjy2r1XXs8xcR-ya3ZcPv0GAAD__3XopJg=
2 changes: 1 addition & 1 deletion pkg/sql/sem/builtins/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ var builtins = map[string]builtinDefinition{
},
),

"repeat": makeBuiltin(tree.FunctionProperties{DistsqlBlacklist: true},
"repeat": makeBuiltin(defProps(),
tree.Overload{
Types: tree.ArgTypes{{"input", types.String}, {"repeat_counter", types.Int}},
ReturnType: tree.FixedReturnType(types.String),
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/sem/tree/datum.go
Original file line number Diff line number Diff line change
Expand Up @@ -1609,7 +1609,7 @@ func (d *DTime) Min(_ *EvalContext) (Datum, bool) {
}

// AmbiguousFormat implements the Datum interface.
func (*DTime) AmbiguousFormat() bool { return false }
func (*DTime) AmbiguousFormat() bool { return true }

// Format implements the NodeFormatter interface.
func (d *DTime) Format(ctx *FmtCtx) {
Expand Down Expand Up @@ -1713,7 +1713,7 @@ func (d *DTimeTZ) Min(_ *EvalContext) (Datum, bool) {
}

// AmbiguousFormat implements the Datum interface.
func (*DTimeTZ) AmbiguousFormat() bool { return false }
func (*DTimeTZ) AmbiguousFormat() bool { return true }

// Format implements the NodeFormatter interface.
func (d *DTimeTZ) Format(ctx *FmtCtx) {
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/sem/tree/format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ func TestFormatExpr(t *testing.T) {
`('Infinity':::DECIMAL + '-Infinity':::DECIMAL) + 'NaN':::DECIMAL`},
{`'+Inf':::FLOAT + '-Inf':::FLOAT + 'NaN':::FLOAT`, tree.FmtParsable,
`('+Inf':::FLOAT + '-Inf':::FLOAT) + 'NaN':::FLOAT`},
{`'12:00:00':::TIME`, tree.FmtParsable,
`'12:00:00':::TIME`},

{`(123:::INT, 123:::DECIMAL)`, tree.FmtCheckEquivalence,
`(123:::INT, 123:::DECIMAL)`},
Expand Down

0 comments on commit 3b4a31c

Please sign in to comment.