Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide Options-like utilities for Hint #251

Open
makenowjust opened this issue Jan 1, 2025 · 2 comments
Open

Provide Options-like utilities for Hint #251

makenowjust opened this issue Jan 1, 2025 · 2 comments

Comments

@makenowjust
Copy link
Collaborator

Motivation

ast.Options has utility methods to access their fields, e.g.,

  • (*ast.Options).Field(name string) (expr Expr, found bool)
  • (*ast.Options).BoolField(name string) (*bool, error)
  • IntegerField and StringField

ast.Hint looks like ast.Options, but it does not have such utilities. Thus, we need to provide such utilities for ast.Hint.

Implementation Ideas

In my thoughts, most of APIs between ast.Options and ast.Hint are shared, so we first introduce a new interface such as:

type Fielder interface {
  func Field(name string) (expr *ast.Expr, found bool)
}

Then, we will implement Fielder for ast.Options and ast.Hint and derive {Bool,Integer,String}Field as Fielder methods.

Questions

  • According to Support path hint keys #242, keys of ast.Hint may be paths. So, is it enough that name is string?
    • IMO, yes, it is enough. name can be dot-chained, and it is resolved in the Field method of each type.
@apstndb
Copy link
Contributor

apstndb commented Jan 1, 2025

We should research about real hints before designing.

  • Hints are less likely to be analyzed by tools than options.
    • The use cases for analyzing in detail are currently solidified in the DDL.
    • Hints can generally be ignored. For example, emulators like handy-spanner will ignore almost all hints.
      • except FORCE_INDEX

        Note: FORCE_INDEX is actually a directive, not a hint, which means an error is raised if the index does not exist.

    • In future, some linters may need to analyze.
  • Generalized BoolField, IntegerField, and StringField may not be useful because:
    • Hints usually utilize enum-like identifiers, not string literals.
    • Generally, hint values are not well typed.

For example, they are popular form of hints. Possible values of OPTIMIER_VERSION hint is 1 to N| latest_version | default_version.

@{OPTIMIZER_VERSION=8} SELECT * FROM MyTable;
@{OPTIMIZER_VERSION=default_version, OPTIMIZER_STATISTICS_PACKAGE=auto_20191128_14_47_22UTC}
SELECT * FROM MyTable;

My current thought:

  • Method Field(name string) (expr *ast.Expr, found bool) for hints may be useful.
    • It can be provided to another [](key, value) like structures, e.g. []NamedArg.
  • dot-chained path as name string may also be useful.
    • I think we can also define (*ast.Path).EqualFold(path string) for this semantics.
  • Because hints and options have significantly different structures, I believe that we need to proceed with caution regarding their unification to define common interface.
  • I don't think there are any real users who find the hint analyze inconvenient, so I think it's okay to lower the priority of this issue and take more time to discuss it.

@makenowjust
Copy link
Collaborator Author

I see. Thank you for response @apstndb.

I agree that additional research is needed and that it is low priority.
We need to discuss to add the Field method to Hint and about (*Path).EqualFold so far.

  • EqualFold seems a bit different topic (And, I think it is really good idea). So, we need another issue for that.
  • Now, I'm not sure what a name is better for the Field method of Hint. There are some names for such a method, e.g., Entry, Value, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants