Skip to content

Commit

Permalink
exec: use same decimal context as non-vec
Browse files Browse the repository at this point in the history 
Use the same decimal contexts that eval.go uses for decimal arithmetic.

Release note: None
  • Loading branch information
@rafiss
rafiss committed Sep 3, 2019
1 parent 75a7258 commit caa3207
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 16 deletions.
30 changes: 18 additions & 12 deletions pkg/sql/exec/execgen/cmd/execgen/overloads.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ var binaryOpDecMethod = map[tree.BinaryOperator]string{
tree.Div: "Quo",
}

var binaryOpDecCtx = map[tree.BinaryOperator]string{
tree.Plus: "ExactCtx",
tree.Minus: "ExactCtx",
tree.Mult: "ExactCtx",
tree.Div: "DecimalCtx",
}

var comparisonOpInfix = map[tree.ComparisonOperator]string{
tree.EQ: "==",
tree.NE: "!=",
Expand Down Expand Up @@ -406,22 +413,21 @@ func (decimalCustomizer) getCmpOpCompareFunc() compareFunc {

func (decimalCustomizer) getBinOpAssignFunc() assignFunc {
return func(op overload, target, l, r string) string {
// todo(jordan): should tree.ExactCtx be used here? (#39540)
if op.BinOp == tree.Div {
return fmt.Sprintf(`
{
cond, err := tree.DecimalCtx.%s(&%s, &%s, &%s)
cond, err := tree.%s.%s(&%s, &%s, &%s)
if cond.DivisionByZero() {
execerror.NonVectorizedPanic(tree.ErrDivByZero)
}
if err != nil {
execerror.NonVectorizedPanic(err)
}
}
`, binaryOpDecMethod[op.BinOp], target, l, r)
`, binaryOpDecCtx[op.BinOp], binaryOpDecMethod[op.BinOp], target, l, r)
}
return fmt.Sprintf("if _, err := tree.DecimalCtx.%s(&%s, &%s, &%s); err != nil { execerror.NonVectorizedPanic(err) }",
binaryOpDecMethod[op.BinOp], target, l, r)
return fmt.Sprintf("if _, err := tree.%s.%s(&%s, &%s, &%s); err != nil { execerror.NonVectorizedPanic(err) }",
binaryOpDecCtx[op.BinOp], binaryOpDecMethod[op.BinOp], target, l, r)
}
}

Expand Down Expand Up @@ -621,7 +627,7 @@ func (c intCustomizer) getBinOpAssignFunc() assignFunc {
// Note that this is the '/' operator, which has a decimal result.
// TODO(rafi): implement the '//' floor division operator.
// todo(rafi): is there a way to avoid allocating on each operation?
// todo(jordan): should tree.ExactCtx be used here? (#39540)
args["Ctx"] = binaryOpDecCtx[op.BinOp]
t = template.Must(template.New("").Parse(`
{
if {{.Right}} == 0 {
Expand All @@ -630,7 +636,7 @@ func (c intCustomizer) getBinOpAssignFunc() assignFunc {
leftTmpDec, rightTmpDec := &apd.Decimal{}, &apd.Decimal{}
leftTmpDec.SetFinite(int64({{.Left}}), 0)
rightTmpDec.SetFinite(int64({{.Right}}), 0)
if _, err := tree.DecimalCtx.Quo(&{{.Target}}, leftTmpDec, rightTmpDec); err != nil {
if _, err := tree.{{.Ctx}}.Quo(&{{.Target}}, leftTmpDec, rightTmpDec); err != nil {
execerror.NonVectorizedPanic(err)
}
}
Expand Down Expand Up @@ -691,13 +697,13 @@ func (c decimalIntCustomizer) getBinOpAssignFunc() assignFunc {
return func(op overload, target, l, r string) string {
isDivision := op.BinOp == tree.Div
args := map[string]interface{}{
"Ctx": binaryOpDecCtx[op.BinOp],
"Op": binaryOpDecMethod[op.BinOp],
"IsDivision": isDivision,
"Target": target, "Left": l, "Right": r,
}
buf := strings.Builder{}
// todo(rafi): is there a way to avoid allocating on each operation?
// todo(jordan): should tree.ExactCtx be used here? (#39540)
t := template.Must(template.New("").Parse(`
{
{{ if .IsDivision }}
Expand All @@ -707,7 +713,7 @@ func (c decimalIntCustomizer) getBinOpAssignFunc() assignFunc {
{{ end }}
tmpDec := &apd.Decimal{}
tmpDec.SetFinite(int64({{.Right}}), 0)
if _, err := tree.DecimalCtx.{{.Op}}(&{{.Target}}, &{{.Left}}, tmpDec); err != nil {
if _, err := tree.{{.Ctx}}.{{.Op}}(&{{.Target}}, &{{.Left}}, tmpDec); err != nil {
execerror.NonVectorizedPanic(err)
}
}
Expand Down Expand Up @@ -764,24 +770,24 @@ func (c intDecimalCustomizer) getBinOpAssignFunc() assignFunc {
return func(op overload, target, l, r string) string {
isDivision := op.BinOp == tree.Div
args := map[string]interface{}{
"Ctx": binaryOpDecCtx[op.BinOp],
"Op": binaryOpDecMethod[op.BinOp],
"IsDivision": isDivision,
"Target": target, "Left": l, "Right": r,
}
buf := strings.Builder{}
// todo(rafi): is there a way to avoid allocating on each operation?
// todo(jordan): should tree.ExactCtx be used here? (#39540)
t := template.Must(template.New("").Parse(`
{
tmpDec := &apd.Decimal{}
tmpDec.SetFinite(int64({{.Left}}), 0)
{{ if .IsDivision }}
cond, err := tree.DecimalCtx.{{.Op}}(&{{.Target}}, tmpDec, &{{.Right}})
cond, err := tree.{{.Ctx}}.{{.Op}}(&{{.Target}}, tmpDec, &{{.Right}})
if cond.DivisionByZero() {
execerror.NonVectorizedPanic(tree.ErrDivByZero)
}
{{ else }}
_, err := tree.DecimalCtx.{{.Op}}(&{{.Target}}, tmpDec, &{{.Right}})
_, err := tree.{{.Ctx}}.{{.Op}}(&{{.Target}}, tmpDec, &{{.Right}})
{{ end }}
if err != nil {
execerror.NonVectorizedPanic(err)
Expand Down
41 changes: 37 additions & 4 deletions pkg/sql/logictest/testdata/logic_test/vectorize
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,39 @@ SELECT a/b, a/c, b/c, b/d, c/d FROM intdecfloat
statement ok
RESET vectorize

# vectorized decimal arithmetic
statement ok
CREATE table decimals (a DECIMAL, b DECIMAL)

statement ok
INSERT INTO decimals VALUES(123.0E200, 12.3)

statement ok
SET vectorize = experimental_always

query R
SELECT a*b FROM decimals
----
1.51290E+203

query R
SELECT a/b FROM decimals
----
1.0E+201

query R
SELECT a+b FROM decimals
----
12300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000012.3

query R
SELECT a-b FROM decimals
----
12299999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999987.7

statement ok
RESET vectorize

# AND expressions.
query II
SELECT a, b FROM a WHERE a < 2 AND b > 0 AND a * b != 3
Expand Down Expand Up @@ -645,14 +678,14 @@ SET tracing = on; SELECT * FROM tpar WHERE a = 0 OR a = 10; SET tracing = off
# tpar, this query will need an adjustment.
query T
SELECT message FROM [SHOW TRACE FOR SESSION] WHERE message IN
('querying next range at /Table/71/1/0',
'querying next range at /Table/71/1/10',
('querying next range at /Table/72/1/0',
'querying next range at /Table/72/1/10',
'=== SPAN START: kv.DistSender: sending partial batch ==='
)
----
querying next range at /Table/71/1/0
querying next range at /Table/72/1/0
=== SPAN START: kv.DistSender: sending partial batch ===
querying next range at /Table/71/1/10
querying next range at /Table/72/1/10

# Test for #38858 -- handle aggregates correctly on an empty table.
statement ok
Expand Down

0 comments on commit caa3207

Please sign in to comment.