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

SELECT 1 runs on server #2104

Closed
vrongmeal opened this issue Nov 15, 2023 · 0 comments · Fixed by #2183
Closed

SELECT 1 runs on server #2104

vrongmeal opened this issue Nov 15, 2023 · 0 comments · Fixed by #2183
Assignees
Labels
bug Something isn't working

Comments

@vrongmeal
Copy link
Contributor

Description

Local:

> select 1;
┌──────────┐
│ Int64(1) │
│       ── │
│    Int64 │
╞══════════╡
│        1 │
└──────────┘

Server logs:

  2023-11-15T13:47:11.392469Z  INFO rpcsrv::handler: executing physical plan, database_id: 00000000-0000-0000-0000-000000000000
    at crates/rpcsrv/src/handler.rs:156 on server-thread-3 ThreadId(5)
    in glaredb::server::rpc_service_request
@vrongmeal vrongmeal added the bug Something isn't working label Nov 15, 2023
@vrongmeal vrongmeal self-assigned this Nov 29, 2023
vrongmeal added a commit that referenced this issue Nov 30, 2023
Fixes: #2104

Signed-off-by: Vaibhav <vrongmeal@gmail.com>
vrongmeal added a commit that referenced this issue Dec 1, 2023
Fixes: #2104

Signed-off-by: Vaibhav <vrongmeal@gmail.com>
vrongmeal added a commit that referenced this issue Dec 4, 2023
Fixes: #2104

Signed-off-by: Vaibhav <vrongmeal@gmail.com>
vrongmeal added a commit that referenced this issue Dec 5, 2023
Fixes: #2104

Signed-off-by: Vaibhav <vrongmeal@gmail.com>
vrongmeal added a commit that referenced this issue Dec 5, 2023
Push down "remote" exec as far down as possible and replace any "local"
execs that are found (children to the remote). Finally, pack the
remote exec in a [`SendRecvJoinExec`] so the plan transformation looks
something like:
    
### Original plan:
    
```txt
                 +-----------+
                 | A (Local) |
                 +-----+-----+
                       |
                       v
              +-----------------+
      +-------+ B (Unspecified) +-------+
      |       +--------+--------+       |
      v                |                v
+------------+         |          +-----------+
| C (Remote) |         |          | F (Local) |
+-----+------+         v          +-----------+
      |          +------------+
      v          | E (Remote) |
+-----------+    +------------+
| D (Local) |
+-----------+
```
    
 ### Transformed plan:

```txt
                          +-----------+
                          | A (Local) |
                          +-----+-----+
                                |
                                v
                       +-----------------+
         +-------------+ B (Unspecified) +-------+
         |             +--------+--------+       |
         |                      |                v
         v                      |          +-----------+
+------------------+            |          | F (Local) |
| SendRecvJoinExec |            v          +-----------+
|                  |   +------------------+
|        C         |   | SendRecvJoinExec |
|        |         |   |                  |
|        v         |   |        E         |
|   D (Send-Recv)  |   +------------------+
+------------------+
```
    
> **Note:** All nodes shown in the graphs above (except `B`) are
> [`RuntimeGroupExec`]s. Other nodes are omitted for simplicity.
    
We don't "pull-up" the remote exec as much as possible because we want
to stop pulling once a "local" node is met but there's no certain way of
knowing that the node is to be run locally (since that information lies
with its n'th parent, i.e., [`RuntimeGroupExec`]).
    
For example, take the following plan in consideration which tries to
copy contents from a remote table in a local file:
    
```txt
RuntimeExec (local)
        |
    CopyToExec
        |
      Limit
        |
RuntimeExec (remote)
        |
    TableScan
```
    
If we were to pull "remote" up until the "local", we would end up with
something like:
    
```txt
RuntimeExec (local)
        |
RuntimeExec (remote)
        |
    CopyToExec
        |
      Limit
        |
    TableScan
```
    
This ends up running the `CopyToExec` on remote which is completely
incorrect.

Fixes: #2104

---------

Signed-off-by: Vaibhav <vrongmeal@gmail.com>
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

Successfully merging a pull request may close this issue.

1 participant