Adds Drop Table

[redixhumayun]

This PR adds support for DROP TABLE and addresses issue #894

It depends on #785 being merged in because it requires the implementation of free_page.
EDIT: The PR above has been merged.

It adds the following:

• an implementation for the DropTable AST instruction via a method called translate_drop_table
• a couple of new instructions - Destroy and DropTable. The former is to modify physical b-tree pages and the latter is to modify in-memory structures like the schema hash table.
• btree_destroy on BTreeCursor to walk the tree of pages for this table and place it in free list.
• state machine traversal for both btree_destroy and clear_overflow_pages to ensure performant, correct code.
• unit & tcl tests
• modifies the Null instruction to follow SQLite semantics and accept a second register. It will set all registers in this range to null. This is required for DROP TABLE.

Bytecode Compatibility

The screenshots below have a comparison of the bytecodes generated via SQLite & Limbo.

Limbo has the same instruction set except for the subroutines which involve opening an ephemeral table, copying over the triggers from the sqlite_schema table and then re-inserting them back into the sqlite_schema table.

This is because OpenEphemeral is still a WIP and is being tracked at #768

[diegoreis42]

Very nice @redixhumayun! I don't understand things below the btree yet but I can give my 2 cents about other things. Idk if you ran EXPLAIN to take a look in the bytecodes, in my machine it produces:

CREATE TABLE a (a);

EXPLAIN DROP TABLE a (a);

addr  opcode         p1    p2    p3    p4             p5  comment
----  -------------  ----  ----  ----  -------------  --  -------------
0     Init           0     37    0                    0   Start at 37
1     Null           0     1     0                    0   r[1]=NULL
2     OpenWrite      0     1     0     5              0   root=1 iDb=0; sqlite_master
3     Rewind         0     11    0                    0
4       Column         0     2     2                    0   r[2]= cursor 0 column 2
5       Ne             3     10    2     BINARY-8       82  if r[2]!=r[3] goto 10
6       Column         0     0     2                    0   r[2]= cursor 0 column 0
7       Eq             4     10    2     BINARY-8       82  if r[2]==r[4] goto 10
8       Rowid          0     5     0                    0   r[5]=sqlite_master.rowid
9       Delete         0     0     0                    2
10    Next           0     4     0                    1
11    Destroy        2     2     0                    0
12    Null           0     6     7                    0   r[6..7]=NULL
13    OpenEphemeral  2     0     6                    0   nColumn=0
14    IfNot          2     22    1                    0
15    OpenRead       1     1     0     4              0   root=1 iDb=0; sqlite_master
16    Rewind         1     22    0                    0
17      Column         1     3     13                   0   r[13]= cursor 1 column 3
18      Ne             2     21    13    BINARY-8       84  if r[13]!=r[2] goto 21
19      Rowid          1     7     0                    0   r[7]= rowid of 1
20      Insert         2     6     7                    0   intkey=r[7] data=r[6]
21    Next           1     17    0                    1
22    OpenWrite      1     1     0     5              0   root=1 iDb=0; sqlite_master
23    Rewind         2     34    0                    0
24      Rowid          2     7     0                    0   r[7]= rowid of 2
25      NotExists      1     33    7                    0   intkey=r[7]
26      Column         1     0     8                    0   r[8]= cursor 1 column 0
27      Column         1     1     9                    0   r[9]= cursor 1 column 1
28      Column         1     2     10                   0   r[10]= cursor 1 column 2
29      Integer        2     11    0                    0   r[11]=2
30      Column         1     4     12                   0   r[12]= cursor 1 column 4
31      MakeRecord     8     5     15    BBBDB          0   r[15]=mkrec(r[8..12])
32      Insert         1     15    7                    0   intkey=r[7] data=r[15]
33    Next           2     24    0                    0
34    DropTable      0     0     0     a              0
35    SetCookie      0     1     2                    0
36    Halt           0     0     0                    0
37    Transaction    0     1     1     0              1   usesStmtJournal=1
38    String8        0     3     0     a              0   r[3]='a'
39    String8        0     4     0     trigger        0   r[4]='trigger'
40    Goto           0     1     0                    0

There are some bytecodes that afaik we don't have implemented yet, like SetCookie, Destroy and DropTable, for OpenEphemeral there is this an open PR #768, but honestly idk how it is used in this case.

[redixhumayun]

@diegoreis42 Hey, thanks for taking a look. Yeah, I took a look at the SQLite bytecode before I started working on this. Although, after your comment I went back fixed a few things.

Yeah, I'm skipping the instructions that don't exist like OpenEphemeral, SetCookie, Destroy etc. However, this should still take care of removing the metadata and freeing up the b-tree pages correctly.

[redixhumayun]

@diegoreis42 @PThorpe92 @jussisaurio Btw how closely are we shooting to match the bytecodes across Limbo & SQLite? Because even for CREATE TABLE, I can see that the byte codes differ quite a bit.

[PThorpe92]

Hey @redixhumayun, I haven't taken a look at this quite yet, but to answer your question, with the exception of the obvious places we will differ like OpenWriteAsync InsertAsync / ..Await, we are aiming for compatibility with sqlite as much as possible. The reason for any statements currently not matching sqlite's output is just due to the amount of opcodes still not implemented and not for any intended difference in design.

I'll have a look at this in the morning 👍

[alpaylan]

In case it uncovers any bugs;
#949

[pereman2]

Maybe not for now but we also need to check overflow pages on interior index pages because they have variable size payload.

[pereman2]

If this returns Locked then we will try to run the traversal again for every overflow page we try to load which is quite inefficient and I don't know if it's safe. It looks like we will need a state machine to keep track of all pages that are being deleted.

[redixhumayun]

@pereman2 Regarding performance concerns, all pages should be in the page cache if the routine resumes, right?. Yes, traversal would be repeated.

I don't understand why it wouldn't be safe though. Could you explain?

Should I go ahead and add a state machine implementation to this routine? Guessing I need to add a new variant to CursorState and use that to track?

[pereman2]

@pereman2 Regarding performance concerns, all pages should be in the page cache if the routine resumes, right?. Yes, traversal would be repeated.

There are no guarantees of a page being in cache unless you pin them. For example, there are no set_dirty or pager.add_dirty calls which marks them as pinned. But even though if we were to pin all of these pages, imagine the case where you drop a table which has 50GB worth of pages, now this means that would have to hold ALL pages in memory, but this is a problem that happens regardless of this destroy stuff anyways. Good thing about a state machine is that you can resume where you started, in this case maybe the PageStack could be enough to ensure we don't traverse again from root to child page every time.

I don't understand why it wouldn't be safe though. Could you explain?

For example you call clear_overflow_pages, which clears the first overflow page, and then it will try to load the second overflow page, but this page could return I/O because it's not in cache. This means that first page is now "freed" and somehow we need to traverse again "first page -> second page" but first page is not there now? This might be also a problem with current clear overflow pages because it should behave like a state machine with DFS.

[redixhumayun]

@pereman2 I've addressed your comments in f2e11a1 and 4eaeb18.
Can you take a look?

It adds a state machine to keep track of Destroy operations and also adds another state machine to keep track of ClearOverflowPage operations.
The reason for two separate state machines is because clear_overflow_pages has two separate call sites (one from btree_destroy and one from delete) and I don't want to complicate the code by linking the state machine used by btree_destroy to the state machine used in clear_overflow_pages