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

Add warning on long query names #787

Merged
merged 2 commits into from Jun 7, 2015
Merged

Add warning on long query names #787

merged 2 commits into from Jun 7, 2015

Conversation

brianc
Copy link
Owner

@brianc brianc commented May 21, 2015

Postgres has a 63 character limit on query names. To avoid potential footgunning of users we'll log to their stderr if they use a longer query name.

For reference: #772

Postgres has a 63 character limit on query names.  To avoid potential footgunning of users we'll log to their stderr if they use a longer query name.

For reference: #772
@bdowning
Copy link

I'll run it against my code when I get back from lunch.

@brianc
Copy link
Owner Author

brianc commented May 21, 2015

Awesome thanks!

@bdowning
Copy link

Looks good to me.

:; ./node_modules/.bin/babel-node --stage 1 --optional bluebirdCoroutines main.js
Listening at http://[::]:3000
process 7459 handling GET /orders/1
Warning! Postgres only supports 63 characters for query names.
You supplied 
        SELECT * FROM (
        WITH matching_orders AS (
            SELECT o.id FROM tix.orders o WHERE o.id = $1
          ), matching_transactions AS (
[...]
             ON at.order_id = o.id
         WHERE o.id IN (SELECT id FROM matching_orders)
         GROUP BY o.id, e.name
    ) e
     ( 1836 )
This can cause conflicts and silent errors executing queries

@bdowning
Copy link

If I'm not mistaken I think you need a check/warning in lib/native too though...

@brianc
Copy link
Owner Author

brianc commented May 21, 2015

Good catch! Also: thanks so much for looking at this. 😄

On Thu, May 21, 2015 at 6:17 PM, Brian Downing notifications@github.com
wrote:

If I'm not mistaken I think you need a check/warning in lib/native too
though...


Reply to this email directly or view it on GitHub
#787 (comment).

@brianc
Copy link
Owner Author

brianc commented Jun 7, 2015

Sorry for the delay on this...added warning for native bindings too!

brianc added a commit that referenced this pull request Jun 7, 2015
@brianc brianc merged commit 711adfe into master Jun 7, 2015
@brianc brianc deleted the warn-on-long-query-names branch June 7, 2015 14:43
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.

None yet

2 participants