Refine QueryExpr and add preliminary support of pipe syntax, only FROM, SELECT, WHERE

[apstndb]

Refine QueryExpr to able to support upcoming pipe syntax.

It requires breaking change of top level query structure.

Breaking changes

• It introduced ast.Query and move WITH, ORDER BY, LIMIT clause into it from ast.QueryStatement and other ast.QueryExprs.
• It introduced ast.AllOrDistinct interface, it replaces distinct in ast.Select.
• It introduced ast.AllOrDistinct typed string. it replaces distinct in ast.Select and ast.CompoundQuery.

Related issues

• close Consider about breaking change to support pipe syntax #199
• close Incompatibility: Paren query expression should able to contain CTE #203
• close Unsupported SELECT ALL #204

[makenowjust]

IMO, this should be type AllOrDistinct string instead, and place it to const.go.

[makenowjust]

Also, this can be replace a Distinct flag in CompoundQuery.

[apstndb]

My thought is that:

• typed string isn't well compatible with current doc comment, sql.go, and ast.go.
  • It spreads logic to many nodes.
• typed string can't prevent wrong value.

[apstndb]

Note: I want to reuse it in pipe operators and GQL statements.

[makenowjust]

It is unclear to me. Please explain it.

• typed string isn't well compatible with current doc comment, sql.go, and ast.go.
  • It spreads logic to many nodes.

No AST node definition is needed to represent one finite type of keyword.
In addition, from my research of the GQL syntax, I am sure that constants of typed strings are sufficient.

Finally, I will make my opinion clear: We MUST use typed string here.

[apstndb]

I agree it is not required to introduce new AST node type, but it can be a design choice so I haven't yet agree your comment.
We should talk about pros/cons.

• typed string isn't well compatible with current doc comment, sql.go, and ast.go.
  • It spreads logic to many nodes.
    It is unclear to me. Please explain it.

1. SQL()
   Currently, Go text/format notation is not needed to be formal, but we need to write SQL() equivalent of

{{if .AllOrDistinct}}{{string(.AllOrDistinct)}} {{end}}

to many places. I think sqlOpt(" ", AllOrDistinct, "") is easy to write.

2. Pos(), End()

Actually, AllOrDistinct is not necessary to define pos and end because it does not appear on node boundaries, but let's think about which one we should use if we were to unify either node interface or typed string.
The general pattern for using typed string is that a separate token.Pos is required to implement pos and end, and their code style are not standarized. (Need to store pos or end?)

memefish/ast/ast.go

Lines 2146 to 2155 in bdde761

	// , INTERLEAVE IN PARENT {{.TableName | sql}} {{.OnDelete}}
	type Cluster struct {
	// pos = Comma
	// end = OnDeleteEnd || TableName.end
	Comma token.Pos // position of ","
	OnDeleteEnd token.Pos // end position of ON DELETE clause
	TableName *Path
	OnDelete OnDeleteAction // optional

memefish/ast/ast.go

Lines 754 to 763 in bdde761

	// {{.Expr | sql}} {{.Collate | sqlOpt}} {{.Direction}}
	type OrderByItem struct {
	// pos = Expr.pos
	// end = DirPos + len(Dir) || (Collate ?? Expr).end
	DirPos token.Pos // position of Dir
	Expr Expr
	Collate *Collate // optional
	Dir Direction // optional

I am doubting that typed string is a good pattern because a single value is no longer self-contained.

[apstndb]

(It is because OnDeleteAction is constructed by multiple token, which can be separated by any whitespaces and comments, so we can't use the first token pos, so it seems that we can also use DirEnd in DirPos's case if needed.)

[makenowjust]

Constant names must be All and Distinct, and zero value is unnecessary.

[apstndb]

I think All is too general a name to bind only AllOrDistinct.
If so, how would you name ALL in the GQL path search prefix?

https://cloud.google.com/spanner/docs/reference/standard-sql/graph-patterns#search_prefix

path_search_prefix:
  {
    ALL
    | ANY
    | ANY SHORTEST
  }

[apstndb]

I'm looking for external guidelines for constants to reference, but I can't find much.
https://google.github.io/styleguide/go/decisions.html#constant-names

Name constants based on their role, not their values. If a constant does not have a role apart from its value, then it is unnecessary to define it as a constant.

// Bad:
const Twelve = 12

const (
    UserNameColumn = "username"
    GroupColumn    = "group"
)

https://go.dev/doc/effective_go (only say about integer constant)

[makenowjust]

Okay, AllOrDistinctAll and AllOrDistinctDistinct is accepted.