From d19f3e81dad0b1940991ba9bf9fe1d346ea010ac Mon Sep 17 00:00:00 2001 From: Bruce Baranowski <92940816+bruce-b-molecula@users.noreply.github.com> Date: Mon, 19 Dec 2022 16:31:27 -0500 Subject: [PATCH] Fb 1818 Implement PREFIX() and SUFFIX() (#2371) * Implement Prefix and Suffix * Update substring out-of-index handling --- sql3/errors.go | 14 ++- sql3/planner/expression.go | 4 + sql3/planner/expressionanalyzercall.go | 5 +- sql3/planner/inbuiltfunctionsstring.go | 80 +++++++++++-- sql3/test/defs/defs_string_functions.go | 152 +++++++++++++++++++++--- 5 files changed, 231 insertions(+), 24 deletions(-) diff --git a/sql3/errors.go b/sql3/errors.go index 937fb45ec..3f4905a34 100644 --- a/sql3/errors.go +++ b/sql3/errors.go @@ -9,8 +9,7 @@ import ( ) const ( - ErrInternal errors.Code = "ErrInternal" - + ErrInternal errors.Code = "ErrInternal" ErrCacheKeyNotFound errors.Code = "ErrCacheKeyNotFound" ErrDuplicateColumn errors.Code = "ErrDuplicateColumn" @@ -114,6 +113,9 @@ const ( // optimizer errors ErrAggregateNotAllowedInGroupBy errors.Code = "ErrIdPercentileNotAllowedInGroupBy" + + // function evaluation + ErrValueOutOfRange errors.Code = "ErrValueOutOfRange" ) func NewErrDuplicateColumn(line int, col int, column string) error { @@ -695,3 +697,11 @@ func NewErrAggregateNotAllowedInGroupBy(line, col int, aggName string) error { fmt.Sprintf("[%d:%d] aggregate '%s' not allowed in GROUP BY", line, col, aggName), ) } + +// function evaluation +func NewErrValueOutOfRange(line, col int, val interface{}) error { + return errors.New( + ErrValueOutOfRange, + fmt.Sprintf("[%d:%d] value '%v' out of range", line, col, val), + ) +} diff --git a/sql3/planner/expression.go b/sql3/planner/expression.go index 52cb9703e..023058e95 100644 --- a/sql3/planner/expression.go +++ b/sql3/planner/expression.go @@ -1500,6 +1500,10 @@ func (n *callPlanExpression) Evaluate(currentRow []interface{}) (interface{}, er return n.EvaluateRTrim(currentRow) case "LTRIM": return n.EvaluateLTrim(currentRow) + case "SUFFIX": + return n.EvaluateSuffix(currentRow) + case "PREFIX": + return n.EvaluatePrefix(currentRow) default: return nil, sql3.NewErrInternalf("unhandled function name '%s'", n.name) } diff --git a/sql3/planner/expressionanalyzercall.go b/sql3/planner/expressionanalyzercall.go index 4eb01a64c..0ee775732 100644 --- a/sql3/planner/expressionanalyzercall.go +++ b/sql3/planner/expressionanalyzercall.go @@ -261,7 +261,10 @@ func (p *ExecutionPlanner) analyzeCallExpression(call *parser.Call, scope parser return p.analyseFunctionTrim(call, scope) case "LTRIM": return p.analyseFunctionTrim(call, scope) - + case "SUFFIX": + return p.analyseFunctionPrefixSuffix(call, scope) + case "PREFIX": + return p.analyseFunctionPrefixSuffix(call, scope) default: return nil, sql3.NewErrCallUnknownFunction(call.Name.NamePos.Line, call.Name.NamePos.Column, call.Name.Name) } diff --git a/sql3/planner/inbuiltfunctionsstring.go b/sql3/planner/inbuiltfunctionsstring.go index a42c6dcf2..28828a74e 100644 --- a/sql3/planner/inbuiltfunctionsstring.go +++ b/sql3/planner/inbuiltfunctionsstring.go @@ -182,8 +182,13 @@ func (n *callPlanExpression) EvaluateSubstring(currentRow []interface{}) (interf if err != nil { return nil, err } + + if startIndex < 0 { + return nil, sql3.NewErrValueOutOfRange(0, 0, startIndex) + } + if startIndex >= len(stringArgOne) { - return "", nil + return nil, sql3.NewErrValueOutOfRange(0, 0, startIndex) } endIndex := len(stringArgOne) @@ -194,16 +199,13 @@ func (n *callPlanExpression) EvaluateSubstring(currentRow []interface{}) (interf } endIndex = startIndex + ln } - if endIndex < 0 { - return "", nil - } - if startIndex < 0 { - startIndex = 0 + if endIndex < startIndex { + return nil, sql3.NewErrValueOutOfRange(0, 0, endIndex) } if endIndex > len(stringArgOne) { - return stringArgOne[startIndex:], nil + return nil, sql3.NewErrValueOutOfRange(0, 0, endIndex) } return stringArgOne[startIndex:endIndex], nil @@ -338,3 +340,67 @@ func (n *callPlanExpression) EvaluateLTrim(currentRow []interface{}) (interface{ // Trim the leading whitespace from string return strings.TrimLeft(stringArgOne, " "), nil } + +func (p *ExecutionPlanner) analyseFunctionPrefixSuffix(call *parser.Call, scope parser.Statement) (parser.Expr, error) { + if len(call.Args) != 2 { + return nil, sql3.NewErrCallParameterCountMismatch(call.Rparen.Line, call.Rparen.Column, call.Name.Name, 2, len(call.Args)) + } + + if !typeIsString(call.Args[0].DataType()) { + return nil, sql3.NewErrStringExpressionExpected(call.Args[0].Pos().Line, call.Args[0].Pos().Column) + } + + if !typeIsInteger(call.Args[1].DataType()) { + return nil, sql3.NewErrIntExpressionExpected(call.Args[1].Pos().Line, call.Args[1].Pos().Column) + } + + call.ResultDataType = parser.NewDataTypeString() + + return call, nil +} + +func (n *callPlanExpression) EvaluatePrefix(currentRow []interface{}) (interface{}, error) { + stringArgOne, err := evaluateStringArg(n.args[0], currentRow) + if err != nil { + return nil, err + } + + intArgTwo, err := evaluateIntArg(n.args[1], currentRow) + if err != nil { + return nil, err + } + + // if the length is less than zero, out of range + if intArgTwo < 0 { + return nil, sql3.NewErrValueOutOfRange(0, 0, intArgTwo) + } + + if intArgTwo > len(stringArgOne) { + return nil, sql3.NewErrValueOutOfRange(0, 0, intArgTwo) + } + + return stringArgOne[:intArgTwo], nil +} + +func (n *callPlanExpression) EvaluateSuffix(currentRow []interface{}) (interface{}, error) { + stringArgOne, err := evaluateStringArg(n.args[0], currentRow) + if err != nil { + return nil, err + } + + intArgTwo, err := evaluateIntArg(n.args[1], currentRow) + if err != nil { + return nil, err + } + + // if the length is less than zero, out of range + if intArgTwo < 0 { + return nil, sql3.NewErrValueOutOfRange(0, 0, intArgTwo) + } + + if intArgTwo > len(stringArgOne) { + return nil, sql3.NewErrValueOutOfRange(0, 0, intArgTwo) + } + + return stringArgOne[len(stringArgOne)-intArgTwo:], nil +} diff --git a/sql3/test/defs/defs_string_functions.go b/sql3/test/defs/defs_string_functions.go index fd75d16e0..59ad9d1ac 100644 --- a/sql3/test/defs/defs_string_functions.go +++ b/sql3/test/defs/defs_string_functions.go @@ -87,26 +87,14 @@ var stringScalarFunctionsTests = TableTest{ SQLs: sqls( "select substring('testing', -10, 14)", ), - ExpHdrs: hdrs( - hdr("", fldTypeString), - ), - ExpRows: rows( - row(string("test")), - ), - Compare: CompareExactUnordered, + ExpErr: "[0:0] value '-10' out of range", }, { name: "SubstringNoLength", SQLs: sqls( "select substring('testing', -5)", ), - ExpHdrs: hdrs( - hdr("", fldTypeString), - ), - ExpRows: rows( - row(string("testing")), - ), - Compare: CompareExactUnordered, + ExpErr: "[0:0] value '-5' out of range", }, { name: "ReverseSubstring", @@ -288,6 +276,142 @@ var stringScalarFunctionsTests = TableTest{ ), ExpErr: "string expression expected", }, + //Prefix() + { + name: "IncorrectArgumentsforPrefix", + SQLs: sqls( + "SELECT PREFIX('string')", + ), + ExpErr: "'PREFIX': count of formal parameters (2) does not match count of actual parameters (1)", + }, + { + name: "IncorrectInputforPrefix", + SQLs: sqls( + "SELECT PREFIX(1,'string')", + ), + ExpErr: "string expression expected", + }, + { + name: "LengthLargerThanStringforPrefix", + SQLs: sqls( + "SELECT PREFIX('string', 7)", + ), + ExpErr: "[0:0] value '7' out of range", + }, + { + name: "NegativeLengthforPrefix", + SQLs: sqls( + "SELECT PREFIX('string', -1)", + ), + ExpErr: "[0:0] value '-1' out of range", + }, + { + name: "ZeroLengthforPrefix", + SQLs: sqls( + "SELECT PREFIX('string', 0)", + ), + ExpHdrs: hdrs( + hdr("", fldTypeString), + ), + ExpRows: rows( + row(string("")), + ), + Compare: CompareExactOrdered, + }, + { + name: "GetFirstThreeforPrefix", + SQLs: sqls( + "SELECT PREFIX('string', 3)", + ), + ExpHdrs: hdrs( + hdr("", fldTypeString), + ), + ExpRows: rows( + row(string("str")), + ), + Compare: CompareExactOrdered, + }, + { + name: "FullStringforPrefix", + SQLs: sqls( + "SELECT PREFIX('string', 6)", + ), + ExpHdrs: hdrs( + hdr("", fldTypeString), + ), + ExpRows: rows( + row(string("string")), + ), + Compare: CompareExactOrdered, + }, + //Suffix() + { + name: "IncorrectArgumentsforSuffix", + SQLs: sqls( + "SELECT SUFFIX('string')", + ), + ExpErr: "'SUFFIX': count of formal parameters (2) does not match count of actual parameters (1)", + }, + { + name: "IncorrectInputforSuffix", + SQLs: sqls( + "SELECT SUFFIX(1,'string')", + ), + ExpErr: "string expression expected", + }, + { + name: "LengthLargerThanStringforSuffix", + SQLs: sqls( + "SELECT SUFFIX('string', 7)", + ), + ExpErr: "[0:0] value '7' out of range", + }, + { + name: "NegativeLengthforSuffix", + SQLs: sqls( + "SELECT SUFFIX('string', -1)", + ), + ExpErr: "[0:0] value '-1' out of range", + }, + { + name: "ZeroLengthforSuffix", + SQLs: sqls( + "SELECT SUFFIX('string', 0)", + ), + ExpHdrs: hdrs( + hdr("", fldTypeString), + ), + ExpRows: rows( + row(string("")), + ), + Compare: CompareExactOrdered, + }, + { + name: "GetFirstThreeforSuffix", + SQLs: sqls( + "SELECT SUFFIX('string', 3)", + ), + ExpHdrs: hdrs( + hdr("", fldTypeString), + ), + ExpRows: rows( + row(string("ing")), + ), + Compare: CompareExactOrdered, + }, + { + name: "FullStringforSuffix", + SQLs: sqls( + "SELECT SUFFIX('string', 6)", + ), + ExpHdrs: hdrs( + hdr("", fldTypeString), + ), + ExpRows: rows( + row(string("string")), + ), + Compare: CompareExactOrdered, + }, { name: "RemovingTrailingspacefromStringusingRTrim", SQLs: sqls(