Fix Children() function of IndexedTableAccess

[andremarianiello]

Fix #478

[reltuk]

If we want to expose the ResolvedTable as a child of the IndexedTableAccess, this is probably going to require more work in the analyzer. Currently the tree walks and the WithChildren calls in things like filter pushdowns are seeing the child of ResolvedTable, and not seeing the ResolvedTable itself. So this change in isolation seems to cause test (and functionality) failures.

[andremarianiello]

@reltuk I was just looking at those, and what you said makes sense. I think a better solution would be to remove the Children and WithChildren methods from IndexedTableTaccess (delegating to the underlying methods from the ResolvedTable), and to stop using the WithChildren method on the IndexedTableAccess node to set the ResolvedTable, like so:

--- a/sql/analyzer/tables.go
+++ b/sql/analyzer/tables.go
@@ -189,7 +189,9 @@ func withTable(node sql.Node, table sql.Table) (sql.Node, error) {
                        if err != nil {
                                return nil, err
                        }
-                       return n.WithChildren(newRt)
+                       n2 := *n
+                       n2.ResolvedTable = newRt
+                       return &n2, nil
                default:
                        return n, nil
                }

What do you think about this solution? It made the sql/analyzer tests pass locally.

[reltuk]

I think that approach could be totally reasonable if it can pass tests. The asymmetry between Children() and WithChildren() currently seems wrong to me. Some of the most comprehensive tests in the project are in //enginetests, so it can be helpful to run those locally to vet changes.

[andremarianiello]

They all passed for me locally, so I'll push these changes.

[reltuk]

Looks great. 👍

[reltuk]

Thanks for the contribution!