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

Return parameters has been declared as table, default kw in declare block, encoding utf8 #27

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

Conversation

kamynina
Copy link

No description provided.

@@ -359,7 +359,7 @@ grammar Piggly
end

rule dollarQuoteMarker
'$' tag:( [a-z\200-\377_] [a-z\200-\377_0-9]* )? '$' <Piggly::Parser::Nodes::TDollarQuoteMarker>
'$' tag:( [a-z\w_] [a-z\w_0-9]* )? '$' <Piggly::Parser::Nodes::TDollarQuoteMarker>
Copy link
Owner

Choose a reason for hiding this comment

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

Perhaps this should this be [[:alpha:]][[:word:]]*? According to https://ruby-doc.org/core-2.1.1/Regexp.html , \w only matches ASCII characters. I'm not sure if PL/pgSQL allows unicode identifiers, but I don't remember why I matched non-ASCII chars here in the first place!

Copy link
Owner

Choose a reason for hiding this comment

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

If Unicode is not allowed, this a-z\w_0-9 could probably be simplified to \w, but the first part should not match numbers, so probably [a-z][\w]* is the right pattern (must start with a letter).

@kputnam
Copy link
Owner

kputnam commented Sep 21, 2016

This looks good! If you test where Unicode is and is not allowed and update the regexps accordingly, I'll merge this.

@dbut023
Copy link
Contributor

dbut023 commented Oct 18, 2016

As it so happens I was just about to submit a PR for table return and WITH queries.
This approach is much simpler; I'd handled param type 't' such that it regenerated the full RETURNS TABLE as originally written which is a bit more intrusive.

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

Successfully merging this pull request may close these issues.

3 participants