restore comments

Author: James Cor

sql/plan/subquery.go

@@ -29,19 +29,34 @@ import (
 // the outer result set. It's in the plan package instead of the expression package because it functions more like a
 // plan Node than an expression.
 type Subquery struct {
-	Query       sql.Node
-	correlated  sql.ColSet
-	hashCache   sql.KeyValueCache
-	b           sql.NodeExecBuilder
+	// The subquery to execute for each row in the outer result set
+	Query sql.Node
+	// correlated is a set of the field references in this subquery from out-of-scope
+	correlated sql.ColSet
+	// Cached hash results, if any
+	hashCache sql.KeyValueCache
+
+	// TODO: convert subquery expressions into apply joins
+	// TODO: move expression.Eval into an execution package
+	// TODO: analyzer rule to connect builder access
+	b sql.NodeExecBuilder
+
+	// Dispose function for the cache, if any. This would appear to violate the rule that nodes must be comparable by
+	// reflect.DeepEquals, but it's safe in practice because the function is always nil until execution.
 	disposeFunc sql.DisposeFunc
 
+	// The original verbatim select statement for this subquery
 	QueryString string
 
-	cache   []interface{}
+	// Cached results, if any
+	cache []interface{}
+	// Mutex to guard the caches
 	cacheMu sync.Mutex
-
-	volatile      bool
+	// Whether results have been cached
 	resultsCached bool
+
+	// volatile indicates that the expression contains a non-deterministic function
+	volatile bool
 }
 
 // NewSubquery returns a new subquery expression.

sql/plan/subqueryalias.go

@@ -33,6 +33,7 @@ type SubqueryAlias struct {
 
 	id sql.TableId
 
+	// expression and is eligible to have visibility to outer scopes of the query.
 	OuterScopeVisibility bool
 	Volatile             bool
 	CacheableCTESource   bool

sql/plan/update.go

@@ -31,12 +31,15 @@ var ErrUpdateUnexpectedSetResult = errors.NewKind("attempted to set field but ex
 // Update is a node for updating rows on tables.
 type Update struct {
 	UnaryNode
-	checks       sql.CheckConstraints
-	Returning    []sql.Expression
-	Ignore       bool
+	checks sql.CheckConstraints
+	// supported in MySQL's syntax, but is exposed through PostgreSQL's syntax.
+	Returning []sql.Expression
+	// IsJoin is true only for explicit UPDATE JOIN queries. It's possible for Update.IsJoin to be false and
+	// Update.Child to be an UpdateJoin since subqueries are optimized as Joins
 	IsJoin       bool
 	HasSingleRel bool
 	IsProcNested bool
+	Ignore       bool
 }
 
 var _ sql.Node = (*Update)(nil)

sql/planbuilder/builder.go

@@ -33,6 +33,8 @@ var BinderFactory = &sync.Pool{New: func() interface{} {
 }}
 
 type Builder struct {
+	// EventScheduler is used to communicate with the event scheduler
+	// for any EVENT related statements. It can be nil if EventScheduler is not defined.
 	scheduler       sql.EventScheduler
 	cat             sql.Catalog
 	authQueryState  sql.AuthorizationQueryState

sql/planbuilder/scope.go

@@ -28,26 +28,45 @@ import (
 // scope tracks relational dependencies necessary to type check expressions,
 // resolve name definitions, and build relational nodes.
 type scope struct {
-	node                sql.Node
-	colset              sql.ColSet
-	ast                 ast.SQLNode
-	exprs               map[string]columnId
-	proc                *procCtx
-	selectAliases       map[string]sql.Expression
-	insertColumnAliases map[string]string
-	parent              *scope
-	activeSubquery      *subquery
-	redirectCol         map[string]scopeColumn
+	node   sql.Node
+	colset sql.ColSet
+	ast    ast.SQLNode
+
+	// exprs collects unique expression ids for reference
+	exprs map[string]columnId
+
+	// tables are the list of table definitions in this scope
 	tables              map[string]sql.TableId
-	ctes                map[string]*scope
-	groupBy             *groupBy
-	b                   *Builder
 	windowDefs          map[string]*sql.WindowDefinition
-	insertTableAlias    string
-	windowFuncs         []scopeColumn
-	extraCols           []scopeColumn
-	cols                []scopeColumn
-	refsSubquery        bool
+	selectAliases       map[string]sql.Expression
+	insertColumnAliases map[string]string
+
+	// redirectCol is used for using and natural joins right-table
+	// attributes that redirect to the left table intersection
+	redirectCol map[string]scopeColumn
+
+	// ctes are common table expressions defined in this scope
+	// TODO: these should be case-sensitive
+	ctes map[string]*scope
+
+	b              *Builder
+	proc           *procCtx
+	parent         *scope
+	activeSubquery *subquery
+	
+	// groupBy collects aggregation functions and inputs
+	groupBy *groupBy
+
+	insertTableAlias string
+
+	// cols are definitions provided by this scope
+	cols []scopeColumn
+	// extraCols are auxillary output columns required for sorting or grouping
+	extraCols []scopeColumn
+	// windowFuncs is a list of window functions in the current scope
+	windowFuncs []scopeColumn
+
+	refsSubquery bool
 }
 
 // resolveColumn matches a variable use to a column definition with a unique

sql/privileges.go

@@ -20,9 +20,9 @@ type PrivilegedOperation struct {
 	Table             string
 	Column            string
 	Routine           string
-	StaticPrivileges  []PrivilegeType
 	DynamicPrivileges []string
-	IsProcedure       bool
+	StaticPrivileges  []PrivilegeType
+	IsProcedure       bool // true if the routine is a procedure, false if it's a function
 }
 
 // PrivilegeCheckSubject is a struct that contains the entity information for an access check. It's specifically what

sql/procedures.go

@@ -44,10 +44,41 @@ type StoredProcedureDetails struct {
 // or deleted by a user. In addition, they're implemented as a function taking standard parameters, compared to stored
 // procedures being implemented as expressions.
 type ExternalStoredProcedureDetails struct {
-	Function  interface{}
-	Name      string
-	Schema    Schema
-	ReadOnly  bool
+	// Function is the implementation of the external stored procedure. All functions should have the following definition:
+	// `func(*Context, <PARAMETERS>) (RowIter, error)`. The <PARAMETERS> may be any of the following types: `bool`,
+	// `string`, `[]byte`, `int8`-`int64`, `uint8`-`uint64`, `float32`, `float64`, `time.Time`, or `Decimal`
+	// (shopspring/decimal). The architecture-dependent types `int` and `uint` (without a number) are also supported.
+	// It is valid to return a nil RowIter if there are no rows to be returned.
+	//
+	// Each parameter, by default, is an IN parameter. If the parameter type is a pointer, e.g. `*int32`, then it
+	// becomes an INOUT parameter. INOUT parameters will be given their zero value if the parameter's value is nil.
+	// There is no way to set a parameter as an OUT parameter.
+	//
+	// Values are converted to their nearest type before being passed in, following the conversion rules of their
+	// related SQL types. The exceptions are `time.Time` (treated as a `DATETIME`), string (treated as a `LONGTEXT` with
+	// the default collation) and Decimal (treated with a larger precision and scale). Take extra care when using decimal
+	// for an INOUT parameter, to ensure that the returned value fits the original's precision and scale, else an error
+	// will occur.
+	//
+	// As functions support overloading, each variant must have a completely unique function signature to prevent
+	// ambiguity. Uniqueness is determined by the number of parameters. If two functions are returned that have the same
+	// name and same number of parameters, then an error is thrown. If the last parameter is variadic, then the stored
+	// procedure functions as though it has the integer-max number of parameters. When an exact match is not found for
+	// overloaded functions, the largest function is used (which in this case will be the variadic function). Also, due
+	// to the usage of the integer-max for the parameter count, only one variadic function is allowed per function name.
+	// The type of the variadic parameter may not have a pointer type.
+	Function interface{}
+	// Name is the name of the external stored procedure. If two external stored procedures share a name, then they're
+	// considered overloaded. Standard stored procedures do not support overloading.
+	Name string
+	// Schema describes the row layout of the RowIter returned from Function.
+	Schema Schema
+	// If true, the procedure is ReadOnly and can be run against a locked or read-only server.
+	ReadOnly bool
+	// If true, then this procedure's access control requires that the user must have explicit Execute permissions
+	// on the procedure in question. If false, then the user will be granted access to the procedure if they have Execute
+	// permissions on the DB. MySQL does not support anything like this, but it is useful for Dolt procedures which
+	// grant elevated access.
 	AdminOnly bool
 }
 

sql/procedures/interpreter_stack.go

@@ -102,7 +102,7 @@ type InterpreterHandler struct {
 	Statement ast.Statement
 	Condition ast.DeclareHandlerConditionValue
 	Action    ast.DeclareHandlerAction
-	Counter   int
+	Counter   int // This is used to track the current position in the stack for the handler
 }
 
 // InterpreterVariable is a variable that lives on the stack.

sql/processlist.go

@@ -92,6 +92,7 @@ const (
 
 // Process represents a process in the SQL server.
 type Process struct {
+	// The time of the last Command transition
 	StartedAt  time.Time
 	Progress   map[string]TableProgress
 	Kill       context.CancelFunc

sql/rowexec/agg.go

@@ -116,9 +116,10 @@ type groupByGroupingIter struct {
 	selectedExprs []sql.Expression
 	groupByExprs  []sql.Expression
 	keys          []uint64
-	keyRow        sql.Row
-	keySch        sql.Schema
-	pos           int
+	// buffers to reduce slice allocations
+	keyRow sql.Row
+	keySch sql.Schema
+	pos    int
 }
 
 func newGroupByGroupingIter(