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

Using visitors you cannot replace elements from a list of values. #107

Closed
gcbirzan opened this issue Jul 28, 2022 · 5 comments
Closed

Using visitors you cannot replace elements from a list of values. #107

gcbirzan opened this issue Jul 28, 2022 · 5 comments

Comments

@gcbirzan
Copy link

Trying to replace them results in an error. Example:

import pglast.ast
from pglast import parse_sql
from pglast.visitors import Visitor


class TestVisitor(Visitor):
    def visit_A_Const(self, ancestors, node):
        return pglast.ast.A_Const(val=pglast.ast.Integer(0))

q = "INSERT INTO foo VALUES (42)"

TestVisitor()(parse_sql(q))

Exception:

Traceback (most recent call last):
  File "scratch_171.py", line 12, in <module>
    TestVisitor()(parse_sql(q))
  File "pglast/visitors.py", line 253, in __call__
    ancestors, node = generator.send(Continue if result is None else result)
  File "pglast/visitors.py", line 322, in iterate
    setattr(parent, ancestors.member, new_node)
TypeError: attribute name must be string, not 'int'

The other sequence I tried with was ARRAY[], where the node in the iterate call is the array, which is a sequence. But, in this case, the node ends up being the actual value, the sequence is a level above, in ancestors.node. I haven't looked enough at the code to be able to suggest a fix, sorry.

@gcbirzan gcbirzan changed the title Using vitors you cannot replace elements from a list of values. Using visitors you cannot replace elements from a list of values. Jul 28, 2022
@lelit
Copy link
Owner

lelit commented Jul 28, 2022

Thanks for the report.
The replace functionality is surely the less tested one, will try to write more tests on it.
I'll be able to study the case this evening.

lelit added a commit that referenced this issue Jul 29, 2022
This is a less naïve way to support AST visitors that apply changes to
the tree, in particular handling the weird case when they alter/remove a
node contained in a (sub)list.

This hopefully fixes issue #107.
@lelit
Copy link
Owner

lelit commented Jul 29, 2022

This seems fixed, can you please give it a try?

@lelit
Copy link
Owner

lelit commented Aug 8, 2022

I just released 3.14 addressing this.

@lelit lelit closed this as completed Aug 8, 2022
@gcbirzan
Copy link
Author

gcbirzan commented Aug 8, 2022

Sorry, I was travelling last week, I didn't get a chance to take a look. It does fix my issue, thank you!

@lelit
Copy link
Owner

lelit commented Aug 8, 2022

Great, thanks for the feedback!

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

No branches or pull requests

2 participants