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

fix: Fix bug when delete multiply rows in one transaction #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

santongding
Copy link

  • Previously, Next after a Delete opcode always end due to the cursor points to an empty node
  • According to https://www.sqlite.org/opcode.html#Delete, when encounter flag OPFLAG_SAVEPOSITION, should move cursor to next or previous record
  • Chosen to move to the previous record, and add a hidden node at the head of the rbtree to avoid complexity

-  Previously, `Next` after a `Delete` opcode always end due to the cursor points to an empty node
- According to https://www.sqlite.org/opcode.html#Delete, when encounter flag `OPFLAG_SAVEPOSITION`, should move cursor to next or previous record
- Chosen to move to the previous record, and add a hidden node at the head of the rbtree to avoid complexity
@santongding
Copy link
Author

Hi! I'm so glad to see such a project. When I was using it, I found some bugs and tried to fix them. Also, I think there should be some test methods to verify the correctness of the transaction results, besides only checking if the transaction executed successfully.

@hananbeer
Copy link
Owner

thank you, this project was an experiment and it is not maintained. feel free to do as you please

@hananbeer
Copy link
Owner

btw this project doesn't aim to maintain 1:1 compatibility. maybe at first but I realized this isn't reasonable in solidity/evm.

there are other interesting approaches to bring sql to blockchains and blockchains to sql.

@santongding
Copy link
Author

btw this project doesn't aim to maintain 1:1 compatibility. maybe at first but I realized this isn't reasonable in solidity/evm.

Whatever, I think this pr is just to fix a bug to ensure the project functions as intended. Beyond that, what this project attracts me is exactly its compatibility, and its a good tutorial for learning bytecode of sqlite, while performance is not a primary concern in EVM.

@ElasticBottle
Copy link

@hananbeer why do you think this isn't reasonable in solity btw?

Aside from the fact that gas might be a lot?

@hananbeer
Copy link
Owner

hananbeer commented Jul 10, 2024

@hananbeer why do you think this isn't reasonable in solity btw?

Aside from the fact that gas might be a lot?

just gas.
if you want to be fully compatible with sqlite then it will cost even more gas, but I don't think this is the right approach. probably equivalency to some extent.

but good news, since then a few good opcodes have been introduced such as TLOAD/TSTORE and MCOPY as well as rollups becoming reality it is quite feasible.
open to collab on this.

@ElasticBottle
Copy link

ElasticBottle commented Jul 11, 2024

What are the right opcodes to be looking at // where did you learn about them?

Do think it's a pretty cool idea that should be unlock by L2/L3s

I think if we can get strings unlocked and joins, this is more than shippable as alpha software

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

Successfully merging this pull request may close these issues.

3 participants