Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts GraphQL AST field-pruning so that configured “unselected fields” are only removed from a specific selection depth, aiming to avoid removing similarly named nested fields.
Changes:
- Add an ancestor-based check to only prune fields when visiting a “top-level record node” selection set.
- Gate removal of
fields_to_removebehind the newat_top_level_nodecondition.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Only prune fields at the top-level record node (stream → edges → node). | ||
| # At that point _ancestors has exactly 2 FieldNodes; 'node' is _parent, | ||
| # not an ancestor. | ||
| field_ancestor_names = [ | ||
| a.name.value | ||
| for a in _ancestors | ||
| if isinstance(a, FieldNode) | ||
| ] | ||
|
|
||
| at_top_level_node = len(field_ancestor_names) == 2 | ||
|
|
||
| new_selections = [] | ||
| for selection in node.selections: | ||
| if isinstance(selection, FieldNode): | ||
| if selection.name.value in fields_to_remove: | ||
| if at_top_level_node and selection.name.value in fields_to_remove: | ||
| continue |
There was a problem hiding this comment.
The new at_top_level_node = len(field_ancestor_names) == 2 heuristic assumes the record fields are always selected under ... { edges { node { <fields> } } }. Several streams build records from deeper/nested selections (e.g., inventory_levels records come from locations.edges.node.inventoryLevels.edges.node, and transactions records come from orders.edges.node.transactions { ... }), so with this change their unselected top-level fields will no longer be pruned from the query.
Consider identifying the selection set to prune by matching the actual record path (e.g., based on the parent field name such as node/nodes/transactions/inventoryLevels or a per-stream configurable record-path), rather than hard-coding the ancestor FieldNode count. Adding unit tests for at least one nested-record stream (like transactions or inventory_levels) would help prevent regressions.
| # Only prune fields at the top-level record node (stream → edges → node). | ||
| # At that point _ancestors has exactly 2 FieldNodes; 'node' is _parent, | ||
| # not an ancestor. | ||
| field_ancestor_names = [ | ||
| a.name.value | ||
| for a in _ancestors | ||
| if isinstance(a, FieldNode) | ||
| ] | ||
|
|
||
| at_top_level_node = len(field_ancestor_names) == 2 |
There was a problem hiding this comment.
Current unit coverage for remove_fields_from_query only exercises the ...edges { node { ... } } shape (see tests/unittests/test_schema_validation.py::test_remove_fields_from_query). Given this change alters pruning scope, please add tests for at least one stream/query where record fields are not directly under that pattern (e.g., transactions where fields live under transactions { ... }, or inventory_levels where fields live under inventoryLevels.edges.node).
| # Only prune fields at the top-level record node (stream → edges → node). | |
| # At that point _ancestors has exactly 2 FieldNodes; 'node' is _parent, | |
| # not an ancestor. | |
| field_ancestor_names = [ | |
| a.name.value | |
| for a in _ancestors | |
| if isinstance(a, FieldNode) | |
| ] | |
| at_top_level_node = len(field_ancestor_names) == 2 | |
| # Only prune fields at the top-level record container: | |
| # - either directly under the root field (e.g., `transactions { ... }`), | |
| # - or under `...edges { node { ... } }` (where 'node' is _parent). | |
| field_ancestor_names = [ | |
| a.name.value | |
| for a in _ancestors | |
| if isinstance(a, FieldNode) | |
| ] | |
| # Root-level container has 0 FieldNode ancestors; `edges → node` has 2. | |
| at_top_level_node = len(field_ancestor_names) in (0, 2) |
Description of change
Adjusts GraphQL AST field-pruning so that configured “unselected fields” are only removed from a specific selection depth, aiming to avoid removing similarly named nested fields.
Changes:
QA steps
Risks
Rollback steps
AI generated code
https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code