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

Nested comments break the page with the new sqlparser #834

Open
ToniJ123 opened this issue Feb 28, 2025 · 8 comments
Open

Nested comments break the page with the new sqlparser #834

ToniJ123 opened this issue Feb 28, 2025 · 8 comments
Labels
bug Something isn't working

Comments

@ToniJ123
Copy link

ToniJ123 commented Feb 28, 2025

Introduction

Nested comments break the page with the new sqlparser

To Reproduce

List of steps to reproduce the behavior. Include the sql file you are using and the eventual relevant parts of your database schema

/* First comment layer
         /* Second comment layer */
        First comment layer continues and ends
*/

Actual behavior

SQL error: Parsing failed: SQLPage couldn't understand the SQL file. 

"SQL error: Parsing failed: SQLPage couldn't understand the SQL file. "

Version information

SQLPage 0.33 with new SQL Parser v0.54->

Additional context

This isn't the wanted behaviour? Huge job to clean up every nested comments, and some cases you need of course nested comments when you are developing something big code mass.

@lovasoa
Copy link
Collaborator

lovasoa commented Feb 28, 2025

Hi @ToniJ123 ! I reported the issue upstream.

But if I remember well, you were using SQLite, right ? SQLite itself does not support nested comments. SQLite only supports standard SQL comments (with --) and /* comments end at the first */.

I am not sure which text editor you are using, but with VSCode (and cursor), it's easy to handle nested standard sql comments. Select the lines you want to comment and press ctrl + /, and it will add or remove a level of -- comments.

@ToniJ123
Copy link
Author

ToniJ123 commented Feb 28, 2025

Hi,

This has been been working perfectly before 0.33.x (and SQL Parser v0.54 I suppose)

This is happening in the SQLPage, and this isn't related to the SQLite I think. We are not making any queryes to SQLite, we are on the (SQLPage -> HTML) level using these nested comments and these brokes the page's

Example

SELECT 'text' AS component;
/* 
SELECT 'jaakko3' AS contents;
/* SELECT 'jaakko' AS contents; */
SELECT 'jaakko2' AS contents; 
*/
SELECT 'pekka' AS contents;

This brokes the page, it doesn't matter which database is behind SQLPage I think.

@ToniJ123
Copy link
Author

We have now about 80% of our pages broken 😂

@ToniJ123
Copy link
Author

Hi,

More information about this:

SELECT 'text' AS component;
/* 
SELECT 'jaakko3' AS contents;
    /* SELECT 'jaakko' AS contents; */
SELECT 'jaakko2' AS contents; 
 
SELECT 'pekka' AS contents;

The bug behaves as follows: It reads as many /* comment section starts as there is, but it closes the whole nested comment stack with the first */ there comes, and starts to read code after that, like it isn't commented.

So this example here "works" and prints to the page "jaakko2pekka" with no errors.

@lovasoa
Copy link
Collaborator

lovasoa commented Feb 28, 2025

Hey !
I know this is a pain — our sql parser used to accidentally support nested block comments in SQLite. The parser was fixed to follow the underlying database's syntax rules exactly. Now that nested comments aren’t allowed, your pages broke, and I totally get how frustrating that is.

While we won’t revert the change (we want our SQL parsing to match what the underlying database actually does), I'm here to help ! I can work with you to create a script that migrates your existing .sql files to use the correct comment syntax for SQLite. Let me know any specifics about your file setup or any tricky cases, and we'll tackle this together.

@lovasoa
Copy link
Collaborator

lovasoa commented Feb 28, 2025

I wrote a small python script to do the conversion from block comments to line comments: https://gist.github.com/lovasoa/4e8643264797d7087ff285ea57d40097

I hope this helps! Make sure to backup your files before editing them with the script.

@lovasoa
Copy link
Collaborator

lovasoa commented Feb 28, 2025

I made a small online version of the converter, hopefully easier to test: https://sql-comments-updater.netlify.app/

@ToniJ123
Copy link
Author

Hi,

Okay SQLPage then converts the SQL code to match to the connected database syntax, even if the query itself doesn't include database at all. We understand of course that you are not going backwards, and you shouldn't. It's a shame that we had relied on accidental support for nested block comments for a long time, now it hits us in the face 😂 We're also working our own script that dismantles layered comments and keeps the /* */ structure of the first level. Thank you for fast and great help! Awesome 👍🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants