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

MULTI does not allow SELECT #4120

Closed
LucHeart opened this issue Nov 13, 2024 · 3 comments · Fixed by #4146
Closed

MULTI does not allow SELECT #4120

LucHeart opened this issue Nov 13, 2024 · 3 comments · Fixed by #4146
Assignees
Labels
bug Something isn't working

Comments

@LucHeart
Copy link

LucHeart commented Nov 13, 2024

Describe the bug
Im trying to use Dragonfly DB with StackExchange.Redis, which is the most popular redis library for C#.
However transactions do not work at all. After some debugging, I figured out the core issue. The issue is that dragonflydb doesnt seem to allow SELECT during MULTI. Dont ask me why it does that, but it does.

The command that SE.Redis sends is for example

SELECT 0 MULTI JSON.SET LoginSession:test . {"Id":"test"} SELECT 0 EXEC

To Reproduce in CLI
Steps to reproduce the behavior:

  1. Start a new transaction with MULTI
  2. SELECT 0 (You can already see the error message)
  3. EXEC to see the final error message that the library also responds with

Expected behavior
SELECT should be allowed in transactions, especially SELECT 0

Screenshots
DragonFly logs
{8486689F-470C-447B-9512-1814CC9F7702}

Wireshark capture
{7D5E22B6-68B2-4246-A05E-2D49432D8AD1}

Environment (please complete the following information):

  • OS: WSL (docker desktop)
  • Kernel: # Command: uname -a
  • Containerized?: Docker
  • Dragonfly Version: 1.25.1

Reproducible Code Snippet

        var transaction = _connectionMultiplexer.GetDatabase().CreateTransaction();
        
        var executeAsync = transaction.ExecuteAsync("JSON.SET", new object[]
        {
            
                "LoginSession:test",
            ".",
            "{\"Id\":\"test\"}"
        });
        await transaction.ExecuteAsync();
        await executeAsync;

Additional context
This also affects libraries that build upon StackExchange.Redis like Redis.OM, this is where I encountered the bug the first time.

@LucHeart LucHeart added the bug Something isn't working label Nov 13, 2024
@romange
Copy link
Collaborator

romange commented Nov 13, 2024

Thanks. Do you know if database id in select can actually change during MULTI?
we can definitely relax this restriction if database id has already been 0, or the same ID before any select in MULTI

@romange
Copy link
Collaborator

romange commented Nov 13, 2024

especially SELECT 0 - indeed, it's a low effort to allow

@LucHeart
Copy link
Author

No I dont think it can change. _connectionMultiplexer.GetDatabase() gets the database you desire. Defaults to 0 here, after that point it cannot switch database anymore.

Well technically you can send a raw query but i dont think we need to worry about that.
Not even sure if multi does support multiple databases in a single transaction

@romange romange self-assigned this Nov 18, 2024
romange added a commit that referenced this issue Nov 18, 2024
romange added a commit that referenced this issue Nov 18, 2024
romange added a commit that referenced this issue Nov 18, 2024
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.

2 participants