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.

Sign up for GitHub

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

Jump to bottom

Trino.Data.ADO: CommandBehavior flags not switched on correctly in ExecuteDbDataReaderAsync #17

Open
stephen-zhao opened this issue Mar 21, 2025 · 2 comments · May be fixed by #22
Open

Trino.Data.ADO: CommandBehavior flags not switched on correctly in ExecuteDbDataReaderAsync #17

stephen-zhao opened this issue Mar 21, 2025 · 2 comments · May be fixed by #22
Assignees
@stephen-zhao

Comments

@stephen-zhao
Copy link
Member

stephen-zhao commented Mar 21, 2025 •
edited
Loading

Issue

I'm experimenting with the Trino.Data.ADO package by plugging it into where we are running a custom Trino ADO client. It seems to be very drop-in-replace, though there's currently an issue.

We are using Dapper to run the queries, and Dapper automatically passes in a CommandBehavior of

CommandBehavior.SequentialAccess | CommandBehavior.SingleResult

This value of CommandBehavior causes TrinoCommand::ExecuteDbDataReaderAsync to throw with a NotSupportedException.

Proposed Solution

It seems the CommandBehavior should be treated as a flags type, and care must be given to parse it out that way when implementing TrinoCommand::ExecuteDbDataReaderAsync. (ADO docs reference)

trino-csharp-client/trino-csharp/Trino.Data.ADO/Server/TrinoCommand.cs

Lines 211 to 230 in 30718ef

switch (behavior)
{
case CommandBehavior.Default:
// Single result means only run one query. Trino only supports one query.
case CommandBehavior.SingleResult:
queryExecutor = await RunQuery().ConfigureAwait(false);
break;
case CommandBehavior.SingleRow:
// Single row requires the reader to be created and the first row to be read.
queryExecutor = await RunQuery().ConfigureAwait(false);
return new TrinoDataReader(queryExecutor);
case CommandBehavior.SchemaOnly:
queryExecutor = await RunNonQuery().ConfigureAwait(false);
break;
case CommandBehavior.CloseConnection:
// Trino has no concept of a connection because every call is a new connection.
queryExecutor = await RunQuery().ConfigureAwait(false);
break;
default:
throw new NotSupportedException();

Instead of a switch, the individual bits should be extracted using bitwise AND test.
@stephen-zhao stephen-zhao self-assigned this Mar 21, 2025
@mosabua
Copy link
Member

mosabua commented Mar 22, 2025

Please send a PR and we can review and merge

@stephen-zhao
Copy link
Member Author

stephen-zhao commented Mar 24, 2025 •
edited
Loading

I'll have something up for this one and the #18 next week (need to pass some internal reviews first)

@stephen-zhao stephen-zhao linked a pull request Mar 26, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@stephen-zhao stephen-zhao

Labels
None yet
Milestone
No milestone
2 participants
@mosabua @stephen-zhao