Skip to content

sqlparser: support stream specific columns#9666

Closed
derekperkins wants to merge 3 commits intovitessio:mainfrom
derekperkins:stream-cols
Closed

sqlparser: support stream specific columns#9666
derekperkins wants to merge 3 commits intovitessio:mainfrom
derekperkins:stream-cols

Conversation

@derekperkins
Copy link
Member

Description

Enable selecting specific columns instead of only stream * from tbl

Related Issue(s)

#6306

Checklist

  • Should this PR be backported? - hoping to get it into v13
  • Tests were added or are not required
  • Documentation was added or is not required

Signed-off-by: Derek Perkins <derek@nozzle.io>
@derekperkins
Copy link
Member Author

I can't get make codegen to run because it won't compile - because of the generated code. What's the workflow to get this to run?

2022/02/10 08:12:54 /Users/derek/go/src/vitess.io/vitess/go/vt/sqlparser/ast_clone.go:1616:37: n.SelectExpr undefined (type *Stream has no field or method SelectExpr)
2022/02/10 08:12:54 /Users/derek/go/src/vitess.io/vitess/go/vt/sqlparser/ast_clone.go:1616:6: out.SelectExpr undefined (type Stream has no field or method SelectExpr)
2022/02/10 08:12:54 /Users/derek/go/src/vitess.io/vitess/go/vt/sqlparser/ast_equals.go:2483:22: a.SelectExpr undefined (type *Stream has no field or method SelectExpr)
2022/02/10 08:12:54 /Users/derek/go/src/vitess.io/vitess/go/vt/sqlparser/ast_equals.go:2483:36: b.SelectExpr undefined (type *Stream has no field or method SelectExpr)
2022/02/10 08:12:54 /Users/derek/go/src/vitess.io/vitess/go/vt/sqlparser/ast_format.go:97:23: node.SelectExpr undefined (type *Stream has no field or method SelectExpr)
2022/02/10 08:12:54 /Users/derek/go/src/vitess.io/vitess/go/vt/sqlparser/ast_format_fast.go:120:7: node.SelectExpr undefined (type *Stream has no field or method SelectExpr)
2022/02/10 08:12:54 /Users/derek/go/src/vitess.io/vitess/go/vt/sqlparser/ast_rewrite.go:4085:37: node.SelectExpr undefined (type *Stream has no field or method SelectExpr)
2022/02/10 08:12:54 /Users/derek/go/src/vitess.io/vitess/go/vt/sqlparser/ast_rewrite.go:4086:20: parent.(*Stream).SelectExpr undefined (type *Stream has no field or method SelectExpr)
2022/02/10 08:12:54 /Users/derek/go/src/vitess.io/vitess/go/vt/sqlparser/ast_visit.go:2026:31: in.SelectExpr undefined (type *Stream has no field or method SelectExpr)
2022/02/10 08:12:54 /Users/derek/go/src/vitess.io/vitess/go/vt/sqlparser/cached_size.go:2112:22: cached.SelectExpr undefined (type *Stream has no field or method SelectExpr)

@GuptaManan100
Copy link
Contributor

Yes, there is a circular dependency, unfortunately right now there is no good way to fix this other than delete those lines from the files (yes the ones which will be generated again and overwritten).

Signed-off-by: Derek Perkins <derek@nozzle.io>
Signed-off-by: Derek Perkins <derek@nozzle.io>
@derekperkins derekperkins added Component: Query Serving release notes Type: Enhancement Logical improvement (somewhere between a bug and feature) labels Feb 10, 2022
@derekperkins derekperkins marked this pull request as ready for review February 10, 2022 16:14
@derekperkins derekperkins marked this pull request as draft February 10, 2022 16:32
Copy link
Contributor

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@derekperkins
Copy link
Member Author

Digging deeper, this doesn't provide much of a win because the whole message row is still loaded into memory, as the message manager can't guarantee that nobody will call stream *. I'll revisit this later with another proposal, probably something around setting visible/invisible columns in the table comments.

@derekperkins
Copy link
Member Author

Thanks to @GuptaManan100 and @systay for helping me debug the ast setup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Query Serving Type: Enhancement Logical improvement (somewhere between a bug and feature)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants