GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — feat(): unicode extended char support for column name or alias by luizzappa · Pull Request #58 · JavaScriptor/js-sql-parser

Conversation

@luizzappa

@luizzappa luizzappa commented Oct 22, 2023

Copy link
Copy Markdown
Contributor

Currently, you cannot name (or alias) a column with certain Latin characters without using quotes, even though this is supported by MySQL syntax. In this PR, I added this.

Parser error:
image

MySQL syntax:

Permitted characters in unquoted identifiers:
- ASCII: [0-9,a-z,A-Z$_] (basic Latin letters, digits 0-9, dollar, underscore)
- Extended: U+0080 .. U+FFFF

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@luizzappa
luizzappa force-pushed the unicode-extended branch 2 times, most recently from 31faa6a to c4b243e Compare October 22, 2023 18:20
@luizzappa

luizzappa commented Oct 22, 2023

Copy link
Copy Markdown
Contributor Author

Sorry for the force, there was code that wasn't supposed to be committed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@albin3 albin3 added the bug label Oct 31, 2023
@albin3

albin3 commented Oct 31, 2023

Copy link
Copy Markdown
Member

hi, @luizzappa thanks for the pr, it is a serious problem. Could you update this pr like this:

  1. I think there is no need to modify lines in src/sqlParser.jison which logic is not modified. It will increase review or blame cost in the future.
  2. It seems [a-zA-Z0-9_\u0080-\uFFFF]* return 'IDENTIFIER' is a better pattern, will fix this issue these days.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
luizzappa added 2 commits November 2, 2023 17:46
@luizzappa
feat(): unicode extended char support for column name or alias
199a1c6
@luizzappa
Merge branch 'master' of github.com:JavaScriptor/js-sql-parser into u…
1064642 
…nicode-extended
@luizzappa
luizzappa force-pushed the unicode-extended branch from 3ff5bc5 to 1064642 Compare November 2, 2023 20:53
@luizzappa

luizzappa commented Nov 2, 2023

Copy link
Copy Markdown
Contributor Author

@albin3, Done. In my first commit I had indented the other lines to maintain the formatting, with this smaller pattern suggested by you I didn't need to do that, thanks.

https://github.com/JavaScriptor/js-sql-parser/compare/master..10646422beaee247db28c171ca791f2880253a19
albin3 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
albin3
albin3 approved these changes Nov 5, 2023
View reviewed changes

@albin3 albin3 left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@albin3

albin3 commented Nov 5, 2023

Copy link
Copy Markdown
Member

hi @luizzappa , lGTM.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@albin3
albin3 merged commit ca3c259 into JavaScriptor:master Nov 5, 2023
@albin3

albin3 commented Apr 3, 2024

Copy link
Copy Markdown
Member

released at [email protected]
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@albin3 albin3 albin3 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@luizzappa @albin3
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.