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

tests: add test to support restart identity and support pgsql-ast-parser@8 #104

Closed

Conversation

RafaelGSS
Copy link
Contributor

This PR aims to support the new feature landed in [email protected]. Shall it be based on next instead of master?

@oguimbal
Copy link
Owner

oguimbal commented Apr 29, 2021

No, master is fine... This project is small so I try too keep things simple.

That said, you might try, but you might have some trouble integrating the breaking change that shipped with your PR I mentioned earlier (types & tests wont be happy if you dont).

If you cant, that's fine. I dont have the time to do it today, but I will try to do it ASAP, so let me know 😀

@RafaelGSS
Copy link
Contributor Author

I'll take a look at it, but probably you will be faster in addressing it.

@RafaelGSS
Copy link
Contributor Author

I took a look but likely I won't have time to work on it right now, several tests are break.

@RafaelGSS
Copy link
Contributor Author

Hello! I will not let it in limbo. I'm planning to work on it in the coming weeks. Probably, I'd like a direction from you somehow. I'll figure out the steps to work on it and come back here for feedback.

@RafaelGSS RafaelGSS force-pushed the tests/add-test-to-truncate-table branch from 58f2d16 to 55c61ac Compare July 12, 2021 18:38
@RafaelGSS RafaelGSS changed the title tests: add test to support restart identity tests: add test to support restart identity and support pgsql-ast-parser@8 Aug 12, 2021
@RafaelGSS
Copy link
Contributor Author

RafaelGSS commented Aug 12, 2021

Hi, I think that I'm almost done. I just need some help to figure out why the truncate tests that I've just added are not working. Perhaps I missed something in oguimbal/pgsql-ast-parser#42 @oguimbal?

I also don't know if the approach that I did is the best one.

@oguimbal
Copy link
Owner

oguimbal commented Aug 16, 2021

Hi, this message:

  1. Deletes
    can truncate table and restart identity:
    Error: 🔨 Not supported 🔨 : The query you ran generated an AST which parts have not been read by the query planner. This means that those parts could be ignored:
⇨ .identity ("restart")

Means that your code has not read the .identity property of the input AST.
This is a security I introduced to silently ignore parts of statements: The query runner gets a proxy to the parsed AST, which ultimately checks that the execution has accessed every property of the AST at least once.

ex: { type: 'select', ...blabla } and { type: 'select', distinct: true, ...blabla} would be executed with the same behaviour if distinct is never read (if, say, we forgot to implement the "distcint" mechanism) 👉 That will not be the case with this mechanism, which will tell you that you forgot to read the distcint property.

ps: Thanks for working on this 🙂 and sorry for my late replies

@RafaelGSS
Copy link
Contributor Author

This is a security I introduced to silently ignore parts of statements: The query runner gets a proxy to the parsed AST, which ultimately checks that the execution has accessed every property of the AST at least once.

I've seen this approach, however, I'm not sure where I should implement it, can you give me the file with an example?

@RafaelGSS RafaelGSS closed this Jan 19, 2023
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.

2 participants