Skip to content

Commit b61e511

Browse files
committed
sql/sem/tree: split derived SemaContext properties from contextual info
Properties derived about expressions during semantic analysis are communicated to callers via ScalarProperties. Prior to this commit, this type was also used to provide contextual information while traversing sub-expressions during semantic analysis. For example, it would indicate whether the current expression is a descendent of a window function expression. These two types of information, derived and contextual, are fundamentally different. Derived properties bubble up from the bottom of the tree to the top, while context propagates downward into sub-expressions. This difference made it difficult to maintaining them correctly in a single type and difficult to reason about. This commit introduces the ScalarScene type which is used for providing internal contextual information during semantic analysis. Release note: None
1 parent 1044a94 commit b61e511

File tree

3 files changed

+65
-42
lines changed

3 files changed

+65
-42
lines changed

pkg/sql/opt/optbuilder/scope.go

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1358,15 +1358,10 @@ func (s *scope) replaceWindowFn(f *tree.FuncExpr, def *tree.ResolvedFunctionDefi
13581358
// We will be performing type checking on expressions from PARTITION BY and
13591359
// ORDER BY clauses below, and we need the semantic context to know that we
13601360
// are in a window function. InWindowFunc is updated when type checking
1361-
// FuncExpr above, but it is reset upon returning from that, so we need to do
1362-
// this update manually.
1363-
defer func(ctx *tree.SemaContext, prevWindow bool) {
1364-
ctx.Properties.Derived.InWindowFunc = prevWindow
1365-
}(
1366-
s.builder.semaCtx,
1367-
s.builder.semaCtx.Properties.Derived.InWindowFunc,
1368-
)
1369-
s.builder.semaCtx.Properties.Derived.InWindowFunc = true
1361+
// FuncExpr above, but it is reset upon returning from that, so we need to
1362+
// do this update manually.
1363+
defer s.builder.semaCtx.Properties.Ancestors.PopTo(s.builder.semaCtx.Properties.Ancestors)
1364+
s.builder.semaCtx.Properties.Ancestors.Push(tree.WindowFuncAncestor)
13701365

13711366
oldPartitions := f.WindowDef.Partitions
13721367
f.WindowDef.Partitions = make(tree.Exprs, len(oldPartitions))

pkg/sql/opt/optbuilder/testdata/aggregate

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2616,7 +2616,7 @@ sort
26162616

26172617
# Grouping columns cannot be reused inside an aggregate input expression
26182618
# because the aggregate input expressions and grouping expressions are
2619-
# built as part of the same projection.
2619+
# built as part of the same projection.
26202620
build
26212621
SELECT max((k+v)/(k-v)) AS r, (k+v)*(k-v) AS s FROM kv GROUP BY k+v, k-v
26222622
----

pkg/sql/sem/tree/type_check.go

Lines changed: 60 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -82,17 +82,21 @@ type SemaContext struct {
8282
// Restore() method, see below.
8383
type SemaProperties struct {
8484
// required constraints type checking to only accept certain kinds
85-
// of expressions. See SetConstraint
85+
// of expressions. See Require.
8686
required semaRequirements
8787

8888
// Derived is populated during semantic analysis with properties
8989
// from the expression being analyzed. The caller is responsible
9090
// for re-initializing this when needed.
9191
Derived ScalarProperties
92+
93+
// Ancestors is mutated during semantic analysis to provide contextual
94+
// information for each descendent during traversal of sub-expressions.
95+
Ancestors ScalarAncestors
9296
}
9397

9498
type semaRequirements struct {
95-
// context is the name of the semantic anlysis context, for use in
99+
// context is the name of the semantic analysis context, for use in
96100
// error messages.
97101
context string
98102

@@ -102,11 +106,14 @@ type semaRequirements struct {
102106
rejectFlags SemaRejectFlags
103107
}
104108

105-
// Require resets the derived properties and sets required constraints.
109+
// Require resets the derived properties and the scalar ancestors, and sets
110+
// required constraints. It must only be called before starting semantic
111+
// analysis and during traversal by semantic analysis itself.
106112
func (s *SemaProperties) Require(context string, rejectFlags SemaRejectFlags) {
107113
s.required.context = context
108114
s.required.rejectFlags = rejectFlags
109115
s.Derived.Clear()
116+
s.Ancestors.clear()
110117
}
111118

112119
// IsSet checks if the given rejectFlag is set as a required property.
@@ -180,23 +187,52 @@ type ScalarProperties struct {
180187
// SeenGenerator is set to true if the expression originally
181188
// contained a SRF.
182189
SeenGenerator bool
183-
184-
// inFuncExpr is temporarily set to true while type checking the
185-
// parameters of a function. Used to process RejectNestedGenerators
186-
// properly.
187-
inFuncExpr bool
188-
189-
// InWindowFunc is temporarily set to true while type checking the
190-
// parameters of a window function in order to reject nested window
191-
// functions.
192-
InWindowFunc bool
193190
}
194191

195192
// Clear resets the scalar properties to defaults.
196193
func (sp *ScalarProperties) Clear() {
197194
*sp = ScalarProperties{}
198195
}
199196

197+
// ScalarAncestors provides context for the current scalar expression during
198+
// semantic analysis. Ancestors are temporarily modified by expressions so that
199+
// their descendent expressions can be analyzed with respect to their ancestors.
200+
type ScalarAncestors byte
201+
202+
const (
203+
// FuncExprAncestor is temporarily added to ScalarAncestors while type
204+
// checking the parameters of a function. Used to process
205+
// RejectNestedGenerators properly.
206+
FuncExprAncestor ScalarAncestors = 1 << iota
207+
208+
// WindowFuncAncestor is temporarily added to ScalarAncestors while type
209+
// checking the parameters of a window function in order to reject nested
210+
// window functions.
211+
WindowFuncAncestor
212+
)
213+
214+
// Push adds the given ancestor to s.
215+
func (s *ScalarAncestors) Push(other ScalarAncestors) {
216+
*s = *s | other
217+
}
218+
219+
// Has returns true if s has the given ancestor.
220+
func (s ScalarAncestors) Has(other ScalarAncestors) bool {
221+
return s&other != 0
222+
}
223+
224+
// PopTo returns s to the given set of ancestors. Use with:
225+
//
226+
// defer semaCtx.Properties.Ancestors.PopTo(semaCtx.Properties.Ancestors)
227+
func (s *ScalarAncestors) PopTo(orig ScalarAncestors) {
228+
*s = orig
229+
}
230+
231+
// clear resets s to the default set of ancestors.
232+
func (s *ScalarAncestors) clear() {
233+
*s = 0
234+
}
235+
200236
// MakeSemaContext initializes a simple SemaContext suitable
201237
// for "lightweight" type checking such as the one performed for default
202238
// expressions.
@@ -966,7 +1002,7 @@ func (sc *SemaContext) checkFunctionUsage(expr *FuncExpr, def *ResolvedFunctionD
9661002
return NewInvalidFunctionUsageError(WindowClass, sc.Properties.required.context)
9671003
}
9681004

969-
if sc.Properties.Derived.InWindowFunc &&
1005+
if sc.Properties.Ancestors.Has(WindowFuncAncestor) &&
9701006
sc.Properties.IsSet(RejectNestedWindowFunctions) {
9711007
return pgerror.Newf(pgcode.Windowing, "window function calls cannot be nested")
9721008
}
@@ -975,7 +1011,7 @@ func (sc *SemaContext) checkFunctionUsage(expr *FuncExpr, def *ResolvedFunctionD
9751011
// If it is an aggregate function *not used OVER a window*, then
9761012
// we have an aggregation.
9771013
if fnCls == AggregateClass {
978-
if sc.Properties.Derived.inFuncExpr &&
1014+
if sc.Properties.Ancestors.Has(FuncExprAncestor) &&
9791015
sc.Properties.IsSet(RejectNestedAggregates) {
9801016
return NewAggInAggError()
9811017
}
@@ -986,7 +1022,7 @@ func (sc *SemaContext) checkFunctionUsage(expr *FuncExpr, def *ResolvedFunctionD
9861022
}
9871023
}
9881024
if fnCls == GeneratorClass {
989-
if sc.Properties.Derived.inFuncExpr &&
1025+
if sc.Properties.Ancestors.Has(FuncExprAncestor) &&
9901026
sc.Properties.IsSet(RejectNestedGenerators) {
9911027
return NewInvalidNestedSRFError(sc.Properties.required.context)
9921028
}
@@ -1080,23 +1116,15 @@ func (expr *FuncExpr) TypeCheck(
10801116
}
10811117

10821118
if semaCtx != nil {
1083-
// We'll need to remember we are in a function application to
1084-
// generate suitable errors in checkFunctionUsage(). We cannot
1085-
// set ctx.inFuncExpr earlier (in particular not before the call
1086-
// to checkFunctionUsage() above) because the top-level FuncExpr
1087-
// must be acceptable even if it is a SRF and
1088-
// RejectNestedGenerators is set.
1089-
defer func(semaCtx *SemaContext, prevFunc bool, prevWindow bool) {
1090-
semaCtx.Properties.Derived.inFuncExpr = prevFunc
1091-
semaCtx.Properties.Derived.InWindowFunc = prevWindow
1092-
}(
1093-
semaCtx,
1094-
semaCtx.Properties.Derived.inFuncExpr,
1095-
semaCtx.Properties.Derived.InWindowFunc,
1096-
)
1097-
semaCtx.Properties.Derived.inFuncExpr = true
1119+
// We'll need to remember we are in a function application to generate
1120+
// suitable errors in checkFunctionUsage(). We cannot enter
1121+
// FuncExprAncestor earlier (in particular not before the call to
1122+
// checkFunctionUsage() above) because the top-level FuncExpr must be
1123+
// acceptable even if it is a SRF and RejectNestedGenerators is set.
1124+
defer semaCtx.Properties.Ancestors.PopTo(semaCtx.Properties.Ancestors)
1125+
semaCtx.Properties.Ancestors.Push(FuncExprAncestor)
10981126
if expr.WindowDef != nil {
1099-
semaCtx.Properties.Derived.InWindowFunc = true
1127+
semaCtx.Properties.Ancestors.Push(WindowFuncAncestor)
11001128
}
11011129
}
11021130

0 commit comments

Comments
 (0)