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.

Sign up for GitHub

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

Jump to bottom

MySQL Parser understands VIEWs with a field list. #87

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

MySQL Parser understands VIEWs with a field list. #87

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
707abe2
MySQL Parser understands VIEWs with a field list.
adherzog Apr 2, 2017
3d090a5
Include 'view' keyword in MySQL Parser generated SQL.
adherzog Apr 10, 2017
8bdf3bf
Improve MySQL view parsing of field list, w/tests.
adherzog Apr 10, 2017
9953926
Simplify setting of column aliases from field list.
adherzog Apr 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/SQL/Translator/Parser/MySQL.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ create : CREATE PROCEDURE NAME not_delimiter "$delimiter"
PROCEDURE : /procedure/i
| /function/i

create : CREATE or_replace(?) create_view_option(s?) /view/i NAME /as/i view_select_statement "$delimiter"
create : CREATE or_replace(?) create_view_option(s?) /view/i NAME parens_field_list(?) /as/i view_select_statement "$delimiter"
Copy link
Member

Choose a reason for hiding this comment

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

An explicit field list overrides the names derived from the SELECT column list, so the field list needs to be saved here and stored in the fields attribute of the SQL::Translator::Schema::View object.

Copy link
Author

Choose a reason for hiding this comment

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

It looks like the fields attribute of the view is generated based on the column aliases, so I'm setting the column aliases rather than explicitly setting the fields attribute.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I misread your statement, ignore the previous comment. It'd be easier to just store the fields in the %views hash and use that if present instead of the alias list in the ->add_view call in sub parse { … }.

Copy link
Author

Choose a reason for hiding this comment

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

If we store the fields in the %views hash instead of setting the column aliases, then the generated SQL won't include the aliases. So I'm continuing to set the aliases, and both the generated SQL and the fields attribute are returning the correct results. (At least, according to the tests I added.)

I've used your simplified code (though I'm using $item{'parens_field_list(?)'}[0] instead of $item{'parens_field_list(?)'}, since it seems to be an arrayref within an arrayref) and left the rest the same.

@ilmari - Please review again when you have a chance.

{
@table_comments = ();
my $view_name = $item{'NAME'};
Expand Down
13 changes: 13 additions & 0 deletions t/02mysql-parser.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,19 @@ BEGIN {
is( join(',', $t2c2->reference_fields), 'id', 'To field "id"' );
}

# Tests for CREATE VIEW statements that contain a column list
# after the view name
{
my $tr = SQL::Translator->new();
my $data = parse($tr,
q[
CREATE
VIEW view_foo (id, name) AS
SELECT id, name FROM thing;
]
) or die $tr->error;
Copy link
Member

Choose a reason for hiding this comment

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

Please also test that the view is in fact correctly parsed and that the column names after the view name take precedence over the ones in the SELECT list.

Copy link
Author

Choose a reason for hiding this comment

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

I've added better tests - it now checks VIEW statements that include 1) a field list, 2) aliases in the SELECT statement, and 3) both a field list and aliases. (When both are specified, I believe the field list should override the column aliases.)

}

# cch Tests for:
# comments like: /*!40101 SET SQL_MODE=@OLD_SQL_MODE */;
# char fields with character set and collate qualifiers
Expand Down