Skip to content

Commit

Permalink
sql: propagate default int size to remote nodes
Browse files Browse the repository at this point in the history
A while ago we added support of `DEFAULT_INT_SIZE` which can be set
either to 8 (default) or 4 bytes. So far that setting has been used to
influence how we parse the "naked" INT in the query from the client.
However, this is one other place where we're performing the parsing:
during expression deserialization on the remote nodes. Previously, the
setting hasn't been propagated to remote nodes, so we might have
interpreted incorrectly. This commit addresses that.

Release note: None
  • Loading branch information
yuzefovich committed Jun 9, 2020
1 parent f0781f2 commit ee33247
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 112 deletions.
7 changes: 1 addition & 6 deletions pkg/sql/conn_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,12 +449,7 @@ func (h ConnectionHandler) GetUnqualifiedIntSize() *types.T {
// no server is actually present.
size = h.ex.sessionData.DefaultIntSize
}
switch size {
case 4, 32:
return types.Int4
default:
return types.Int
}
return parser.NakedIntTypeFromDefaultIntSize(size)
}

// GetParamStatus retrieves the configured value of the session
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/distsql/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,8 @@ func (ds *ServerImpl) setupFlow(
BytesEncodeFormat: be,
ExtraFloatDigits: int(req.EvalContext.ExtraFloatDigits),
},
VectorizeMode: sessiondata.VectorizeExecMode(req.EvalContext.Vectorize),
VectorizeMode: sessiondata.VectorizeExecMode(req.EvalContext.Vectorize),
DefaultIntSize: int(req.EvalContext.DefaultIntSize),
}
ie := &lazyInternalExecutor{
newInternalExecutor: func() sqlutil.InternalExecutor {
Expand Down
5 changes: 4 additions & 1 deletion pkg/sql/execinfra/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ func processExpression(
if exprSpec.Expr == "" {
return nil, nil
}
expr, err := parser.ParseExpr(exprSpec.Expr)
expr, err := parser.ParseExprWithInt(
exprSpec.Expr,
parser.NakedIntTypeFromDefaultIntSize(evalCtx.SessionData.DefaultIntSize),
)
if err != nil {
return nil, err
}
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/execinfrapb/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func MakeEvalContext(evalCtx *tree.EvalContext) EvalContext {
BytesEncodeFormat: be,
ExtraFloatDigits: int32(evalCtx.SessionData.DataConversion.ExtraFloatDigits),
Vectorize: int32(evalCtx.SessionData.VectorizeMode),
DefaultIntSize: int32(evalCtx.SessionData.DefaultIntSize),
}

// Populate the search path. Make sure not to include the implicit pg_catalog,
Expand Down
209 changes: 117 additions & 92 deletions pkg/sql/execinfrapb/api.pb.go

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion pkg/sql/execinfrapb/api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,14 @@ message EvalContext {
optional string location = 4 [(gogoproto.nullable) = false];
optional string database = 5 [(gogoproto.nullable) = false];
repeated string search_path = 6;
optional string temporary_schema_name = 13 [(gogoproto.nullable) = false];
optional string user = 7 [(gogoproto.nullable) = false];
optional SequenceState seq_state = 8 [(gogoproto.nullable) = false];
optional string application_name = 9 [(gogoproto.nullable) = false];
optional BytesEncodeFormat bytes_encode_format = 10 [(gogoproto.nullable) = false];
optional int32 extra_float_digits = 11 [(gogoproto.nullable) = false];
optional int32 vectorize = 12 [(gogoproto.nullable) = false];
optional string temporary_schema_name = 13 [(gogoproto.nullable) = false];
optional int32 default_int_size = 14 [(gogoproto.nullable) = false];
}

// BytesEncodeFormat is the configuration for bytes to string conversions.
Expand Down
47 changes: 36 additions & 11 deletions pkg/sql/parser/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,22 @@ type Parser struct {

// INT8 is the historical interpretation of INT. This should be left
// alone in the future, since there are many sql fragments stored
// in various descriptors. Any user input that was created after
// in various descriptors. Any user input that was created after
// INT := INT4 will simply use INT4 in any resulting code.
var defaultNakedIntType = types.Int

// NakedIntTypeFromDefaultIntSize given the size in bits or bytes (preferred)
// of how a "naked" INT type should be parsed returns the corresponding integer
// type.
func NakedIntTypeFromDefaultIntSize(defaultIntSize int) *types.T {
switch defaultIntSize {
case 4, 32:
return types.Int4
default:
return types.Int
}
}

// Parse parses the sql and returns a list of statements.
func (p *Parser) Parse(sql string) (Statements, error) {
return p.parseWithDepth(1, sql, defaultNakedIntType)
Expand All @@ -103,8 +115,8 @@ func (p *Parser) ParseWithInt(sql string, nakedIntType *types.T) (Statements, er
return p.parseWithDepth(1, sql, nakedIntType)
}

func (p *Parser) parseOneWithDepth(depth int, sql string) (Statement, error) {
stmts, err := p.parseWithDepth(1, sql, defaultNakedIntType)
func (p *Parser) parseOneWithInt(sql string, nakedIntType *types.T) (Statement, error) {
stmts, err := p.parseWithDepth(1, sql, nakedIntType)
if err != nil {
return Statement{}, err
}
Expand Down Expand Up @@ -232,8 +244,14 @@ func Parse(sql string) (Statements, error) {
// bits of SQL from other nodes. In general, we expect that all
// user-generated SQL has been run through the ParseWithInt() function.
func ParseOne(sql string) (Statement, error) {
return ParseOneWithInt(sql, defaultNakedIntType)
}

// ParseOneWithInt is similar to ParseOne but interprets the INT and SERIAL
// types as the provided integer type.
func ParseOneWithInt(sql string, nakedIntType *types.T) (Statement, error) {
var p Parser
return p.parseOneWithDepth(1, sql)
return p.parseOneWithInt(sql, nakedIntType)
}

// ParseQualifiedTableName parses a SQL string of the form
Expand Down Expand Up @@ -262,9 +280,9 @@ func ParseTableName(sql string) (*tree.UnresolvedObjectName, error) {
return rename.Name, nil
}

// parseExprs parses one or more sql expressions.
func parseExprs(exprs []string) (tree.Exprs, error) {
stmt, err := ParseOne(fmt.Sprintf("SET ROW (%s)", strings.Join(exprs, ",")))
// parseExprsWithInt parses one or more sql expressions.
func parseExprsWithInt(exprs []string, nakedIntType *types.T) (tree.Exprs, error) {
stmt, err := ParseOneWithInt(fmt.Sprintf("SET ROW (%s)", strings.Join(exprs, ",")), nakedIntType)
if err != nil {
return nil, err
}
Expand All @@ -275,17 +293,24 @@ func parseExprs(exprs []string) (tree.Exprs, error) {
return set.Values, nil
}

// ParseExprs is a short-hand for parseExprs(sql)
// ParseExprs is a short-hand for parseExprs(sql).
func ParseExprs(sql []string) (tree.Exprs, error) {
if len(sql) == 0 {
return tree.Exprs{}, nil
}
return parseExprs(sql)
return parseExprsWithInt(sql, defaultNakedIntType)
}

// ParseExpr is a short-hand for parseExprs([]string{sql})
// ParseExpr is a short-hand for parseExprsWithInt([]string{sql},
// defaultNakedIntType).
func ParseExpr(sql string) (tree.Expr, error) {
exprs, err := parseExprs([]string{sql})
return ParseExprWithInt(sql, defaultNakedIntType)
}

// ParseExprWithInt is a short-hand for parseExprsWithInt([]string{sql},
// nakedIntType).'
func ParseExprWithInt(sql string, nakedIntType *types.T) (tree.Expr, error) {
exprs, err := parseExprsWithInt([]string{sql}, nakedIntType)
if err != nil {
return nil, err
}
Expand Down

0 comments on commit ee33247

Please sign in to comment.