这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@0x777
Copy link
Member

@0x777 0x777 commented Oct 11, 2018

Description

Postgres (with default setting) limits the length of an identifier to 63 characters (to be precise it ignores anything beyond 63 characters). Given that we generate readable aliases for queries (which could potentially exceed 63 characters because of nesting) since alpha23 sometimes this results in a postgres query error.

What component does this PR affect?

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Requires changes from other components? If yes, please mark the components:

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Related Issue

#688

Solution and Design

A new Rewrite module traverses the generated SQL AST and prefixes each alias with an integer. Even if Postgres ignores anything beyond 63 characters, the uniqueness of the alias is guaranteed by the prefixed integer.

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs update
  • Community content

Checklist:

  • I have read the contributing guide and my code conforms to the guidelines.
  • This change requires a change in the documentation.
  • I have updated the documentation accordingly.
  • I have added required tests.

@CLAassistant
Copy link

CLAassistant commented Oct 11, 2018

CLA assistant check
All committers have signed the CLA.

@0x777 0x777 requested a review from rakeshkky October 11, 2018 12:09
rakeshkky
rakeshkky previously approved these changes Oct 11, 2018
@shahidhk shahidhk changed the title work around postgres's default limit of 63 chars for identifiers, closes #688 workaround postgres default limit of 63 chars for identifiers (close #688) Oct 12, 2018
@shahidhk shahidhk added k/bug Something isn't working c/server Related to server labels Oct 12, 2018
@shahidhk
Copy link
Member

@0x777 @rakeshkky Shouldn't we add a few test-cases for this?

@shahidhk shahidhk merged commit ecf8c76 into master Oct 12, 2018
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-707.herokuapp.com is deleted

@0x777 0x777 deleted the issue-688 branch October 16, 2018 09:18
hasura-bot pushed a commit that referenced this pull request Jun 25, 2024
## Description

According to NDC headers pass-through spec, commands can include headers
in their responses, which are forwarded as response headers by the
engine to the client. This PR implements it.

V3_GIT_ORIGIN_REV_ID: 4fe458db02c5dd51f4674e4e013312f8e179c087
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c/server Related to server k/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants