-
Notifications
You must be signed in to change notification settings - Fork 115
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
Fix %%model
and %model
ipython magic error for seed model with csv column name with periods
#2634
Fix %%model
and %model
ipython magic error for seed model with csv column name with periods
#2634
Conversation
weird - the tests are failing on stuff I didn't touch. I thought removing the |
Hey @butchland, rebasing off of main should fix the CI issues. Can you share some context around this fix? What exactly was the issue / error you saw? |
… model ipython magic tests
d668581
to
6e4dcba
Compare
hi @georgesittas sorry for the late reply - here is a colab notebook showing the error that occurs when the seed data contains a csv file with a column name that has a period in it.. |
I see, thanks for sharing the notebook @butchland. I understand the issue now. I don't think this approach to solving it is correct, though. Seeds with dots in the column names are perfectly legal, and we shouldn't just drop that information to make the parser happy. Also, this doesn't address similar issues with other "invalid" identifier characters. The issue here is that we're trying to parse these names as if they are valid SQL, even though that's not really the case. The proper way to handle them is to parse them into identifiers, and if they contain any "special" characters, ensure that they are quoted. That way, we'll convert them to valid SQL. |
I'll go ahead and close this PR for now, but I've opened a ticket to make sure this is tracked. We'll take a look soon, thanks for bringing this into our attention and appreciate the PR! |
%model
and%%model
line magic