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

go-dqlite doesn't support nested or interleaved Query #291

Open
cole-miller opened this issue Mar 15, 2024 · 4 comments
Open

go-dqlite doesn't support nested or interleaved Query #291

cole-miller opened this issue Mar 15, 2024 · 4 comments

Comments

@cole-miller
Copy link
Contributor

@ycliuhw pointed out (MM) that go-dqlite can't handle running a query on a Tx while in the middle of iterating over the rows from a previous query on the same Tx, thus:

    // Prepare the first query
    rows, err := tx.Query("SELECT id FROM table1")
    if err != nil {
        log.Fatal(err)
    }
    defer rows.Close()

    for rows.Next() {
        var id int
        if err := rows.Scan(&id); err != nil {
            log.Fatal(err)
        }

        // Inside the loop, initiate a new query based on the current row's data
        innerRows, err := tx.Query("SELECT info FROM table2 WHERE related_id = ?", id)
        if err != nil {
            log.Fatal(err)
        }
        
        for innerRows.Next() {
            var info string
            if err := innerRows.Scan(&info); err != nil {
                log.Fatal(err)
            }
            fmt.Println("Info:", info)
        }
        innerRows.Close() // Make sure to close the inner cursor
    }

I think this is ultimately due to limitations in the wire protocol but need to think through it a bit more.

@freeekanayaka
Copy link
Contributor

It should be possible to tweak the wire protocol (and/or its implementation) to accommodate this use case, and do it in a backward-compatible way.

However, may I ask what is the real-world situation where the above pattern is desired? Normally it's much more efficient to avoid this N * M pattern and turn it into an N + M pattern, or even better just N, e.g.:

  • use a join (turning this into an N pattern)
  • fully iterate the first result set, collecting the all IDs or data you need, and then issue a single (or more, if the data is not homogeneous) bulk queries collecting all the additional data at once (N + M pattern)

So while it's true that this can't currently be done in dqlite, I tend to think that in most cases (if not all) this pattern leads to less-than-optimal data access.

@tomponline
Copy link
Member

So while it's true that this can't currently be done in dqlite, I tend to think that in most cases (if not all) this pattern leads to less-than-optimal data access.

Tend to agree too. In the past when we've hit this issue in LXD its been down to a bug or accidental nested transaction rather than a desirable scenario.

@cole-miller
Copy link
Contributor Author

Sorry, just circling back to this now. When this was brought up on MM the person reporting it noted that yes, this can and probably should be done with a join, but also that the mattn/go-sqlite3 driver does support this kind of interleaving.

So probably the best thing to do here is to find a place to document that go-dqlite doesn't support this. Eventually we will start tagging requests and responses in the wire protocol with unique IDs (since there are other good reasons to do this) and I think that should also unlock support for this feature, but it doesn't seem like supporting said feature is a compelling reason to increase priority for that task.

@SimonRichardson
Copy link
Member

@cole-miller I would agree this is a low priority. The statement has been reworked to no longer do this.

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

4 participants