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

Deparse SELECT NULL and BooleanTest. #57

Merged
merged 5 commits into from
Oct 19, 2016

Conversation

jcsjcs
Copy link
Contributor

@jcsjcs jcsjcs commented Oct 18, 2016

These changes to the deparser handle:
SELECT NULL
WHERE a IS true
WHERE a is false
WHERE a IS NOT true
WHERE a IS NOT false

when BOOLEAN_TEST_FALSE
' IS false'
when BOOLEAN_TEST_NOT_FALSE
' IS NOT false'
Copy link
Member

Choose a reason for hiding this comment

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

Given that https://www.postgresql.org/docs/9.5/static/functions-comparison.html has true/false uppercase (i.e. TRUE/FALSE), I'd be more inclined to deparse to uppercase as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I'll correct.

@jcsjcs
Copy link
Contributor Author

jcsjcs commented Oct 18, 2016

Looking at https://www.postgresql.org/docs/9.5/static/functions-comparison.html, I think I should add the deparsing of IS UNKNOWN and IS NOT UNKNOWN at well - for completeness sake.

@lfittl
Copy link
Member

lfittl commented Oct 18, 2016

@jcsjcs Good point - do you want to add that to this PR as well?

BOOLEAN_TEST_FALSE => ' IS FALSE',
BOOLEAN_TEST_NOT_FALSE => ' IS NOT FALSE',
BOOLEAN_TEST_UNKNOWN => ' IS UNKNOWN',
BOOLEAN_TEST_NOT_UNKNOWN => ' IS NOT UNKNOWN' }
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for one more nitpick - since this variable never changes, we could move it on top of the method and freeze it, to save an allocation, like this:

BOOLEAN_TEST_TYPE_TO_STRING = {
  BOOLEAN_TEST_TRUE        => ' IS TRUE',
  BOOLEAN_TEST_NOT_TRUE    => ' IS NOT TRUE',
  BOOLEAN_TEST_FALSE       => ' IS FALSE',
  BOOLEAN_TEST_NOT_FALSE   => ' IS NOT FALSE',
  BOOLEAN_TEST_UNKNOWN     => ' IS UNKNOWN',
  BOOLEAN_TEST_NOT_UNKNOWN => ' IS NOT UNKNOWN'
}.freeze
def deparse_boolean_test(node)
  deparse_item(node['arg']) + BOOLEAN_TEST_TYPE_TO_STRING[node['booltesttype']]
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem, done.

@lfittl lfittl merged commit f388f09 into pganalyze:master Oct 19, 2016
@jcsjcs jcsjcs deleted the deparse_null_booltest branch October 19, 2016 10:07
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.

2 participants