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 — Allow specific keywords to be parsed as identifiers (fix issue with RENAME COLUMN) by minleejae · Pull Request #2203 · JSQLParser/JSqlParser

Conversation

@minleejae

@minleejae minleejae commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

Summary

This PR introduces a new parser rule KeywordOrIdentifier() to handle cases where certain keywords are used as identifiers. This addresses issues that arose after specific tokens like NAME were introduced as keywords.

Problem

While parsing a statement like:

ALTER TABLE test_table RENAME COLUMN name TO full_name;

the parser failed because name was recognized strictly as a <K_NAME> keyword after this commit, instead of being allowed as an identifier.

Solution

  • Introduced KeywordOrIdentifier() rule that accepts:
    • <S_IDENTIFIER>
    • <S_QUOTED_IDENTIFIER>
    • Selected keywords like <K_NAME>, <K_NEXT>, <K_VALUE>, <K_PUBLIC>, <K_STRING>
    • Additional tokens can be added as needed in the future.
  • Replaced relevant parts of the grammar to use KeywordOrIdentifier().

I’m open to any feedback or suggestions on this approach!

Sorry, something went wrong.

Uh oh!

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

All reactions
@minleejae
Add KeywordOrIdentifier rule to support keywords as identifiers
586d597
@manticore-projects

manticore-projects commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

We need to rework the whole keyword management eventually. But in the meantime this can be a remedy.

minleejae 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.

@manticore-projects
manticore-projects merged commit a068dd4 into JSQLParser:master Mar 24, 2025
@minleejae

minleejae commented Mar 24, 2025

Copy link
Copy Markdown
Contributor Author

Hi, @manticore-projects
I also agree that the current keyword handling could use a more systematic approach.
Out of curiosity, could you share your thoughts on what the ultimate direction for keyword management should look like? I'd like to align any future refactoring efforts accordingly.

All reactions

Sorry, something went wrong.

Uh oh!

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

@manticore-projects

manticore-projects commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

The current implementation has grown more or less wild over 20 years and based on demand. It also suffers limitations from JavaCC 7.
At the same time it somehow just works good enough for most use-cases and rewriting it will take a lot of effort.

So my current idea was:

  1. switch to JavaCC 8 (or even CongoCC)
  2. then rework the DDL statements (they are a big mess)
  3. then rewrite the KEYWORD handling
minleejae 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.

@minleejae

minleejae commented Mar 24, 2025

Copy link
Copy Markdown
Contributor Author

Hi, @manticore-projects
This sounds like quite a large-scale refactoring effort. I’m curious — do you have any rough timeline or plan in mind for when you'd like to tackle this?

Also, just wondering, have you ever considered adopting something like ANTLR instead, or is the plan to stick with JavaCC/CongoCC going forward?

All reactions

Sorry, something went wrong.

Uh oh!

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

@manticore-projects

manticore-projects commented Mar 24, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

ANTLR has been found producing slower code and so is out of scope for me. Although this claim could need a verification again.

Also, when rebasing to ANTLR then we would/should have to rewrite from scratch.

All reactions

Sorry, something went wrong.

Uh oh!

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

@minleejae

minleejae commented Mar 25, 2025

Copy link
Copy Markdown
Contributor Author

@manticore-projects
It's interesting that ANTLR was found to be slow. I wonder if that has something to do with the structural differences between LL and LR parsers. Of course, replacing ANTLR would come with the high cost of building everything from scratch, but I thought one of its strengths was the ability to support multiple languages.

All reactions

Sorry, something went wrong.

Uh oh!

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

@manticore-projects

manticore-projects commented Mar 25, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

I am not doubting the strength or superiority of ANTLR in general but only point out why I was more interested in sticking with the JavaCC origin as long as possible. JavaCC/CongoCC also supports multiple languages although I am really interested in Java only (and maybe C).

For my own purpose (real time parsing SQL while typing) I am interested in the fastest (automatically generated) parser possible -- and so far JavaCC holds that position.

minleejae 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.

@minleejae
minleejae deleted the fix/rename branch April 2, 2025 09:16
@willneedham93 willneedham93 mentioned this pull request Nov 13, 2025
[BUG] JSQLParser 5.3 : PostgreSQL : Cannot parse DROP COLUMN command where data is the column name #2339
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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

@minleejae @manticore-projects
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.