Skip to content

feat: Spanner non-awaiting DDL #15280

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

Merged
amanda-tarafa merged 5 commits into from
Jan 13, 2026
Merged

feat: Spanner non-awaiting DDL #15280

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1aa3b4f
feat: Spanner non-awaiting DDL
olavloite Nov 28, 2025
c299d51
Merge branch 'main' into spanner-non-blocking-ddl
olavloite Nov 28, 2025
5752bb3
chore: refactor to simply use await
olavloite Nov 28, 2025
3c984c9
Merge branch 'main' into spanner-non-blocking-ddl
olavloite Dec 15, 2025
901a44e
chore: address review comments
olavloite Jan 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data.IntegrationTests/AdminTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,17 +239,19 @@ AlbumTitle STRING(MAX),
using (var connection = new SpannerConnection(builder.WithDatabase(dbName)))
{
var dropSingersCmd = connection.CreateDdlCommand("DROP TABLE Singers");
var dropAlbumsCmd = connection.CreateDdlCommand("DROP TABLE Albums");
var dropBothTablesCmd = connection.CreateDdlCommand("DROP TABLE Albums", "DROP TABLE Singers");

await Assert.ThrowsAsync<SpannerException>(() => dropSingersCmd.ExecuteNonQueryAsync());
await dropAlbumsCmd.ExecuteNonQueryAsync();
await dropSingersCmd.ExecuteNonQueryAsync();
var operationName = await dropBothTablesCmd.StartDdlAsync();
Assert.NotEqual("", operationName);
Copy link
Contributor

@amanda-tarafa amanda-tarafa Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind leaving the existing tests as is and creating a new one for this. The one we have is not great (re: name and what its testing) ,but if we also check that the operation name is returned, then it gets even more confusing. I'm fine if you duplicate this one and tweak it, or just use the create tables and drop database parts of it.

Copy link
Contributor Author

@olavloite olavloite Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}

using (var connection = new SpannerConnection(builder))
{
var dropCommand = connection.CreateDdlCommand($"DROP DATABASE {dbName}");
await dropCommand.ExecuteNonQueryAsync();
var operationName = await dropCommand.StartDdlAsync();
// DropDatabase does not return a long-running operation.
Assert.Equal("", operationName);
}
}
}
Expand Down
32 changes: 29 additions & 3 deletions apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data/SpannerCommand.ExecutableCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using Google.Cloud.Spanner.Admin.Database.V1;
using Google.Cloud.Spanner.Common.V1;
using Google.Cloud.Spanner.V1;
using Google.LongRunning;
using Google.Protobuf;
using Google.Protobuf.WellKnownTypes;
using Grpc.Core;
Expand Down Expand Up @@ -235,10 +236,27 @@ private async Task<SpannerDataReader> ExecuteDmlReaderAsync(CommandBehavior beha
}

private async Task<int> ExecuteDdlAsync(CancellationToken cancellationToken)
{
await ExecuteDdlAsync(pollUntilCompleted: true, cancellationToken).ConfigureAwait(false);
return 0;
}

/// <summary>
/// Starts a DDL operation, but does not wait for the long-running operation to finish.
/// </summary>
/// <returns>The name of the long-running operation that was created</returns>
internal async Task<string> StartDdlAsync(CancellationToken cancellationToken)
{
var operation = await ExecuteDdlAsync(pollUntilCompleted: false, cancellationToken).ConfigureAwait(false);
return operation?.Name ?? "";
Copy link
Contributor

@amanda-tarafa amanda-tarafa Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe return null instead of ""?

Copy link
Contributor Author

@olavloite olavloite Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}

private async Task<Operation> ExecuteDdlAsync(bool pollUntilCompleted, CancellationToken cancellationToken)
{
string commandText = CommandTextBuilder.CommandText;
var builder = Connection.Builder;
var connectionOptions = new SpannerClientCreationOptions(builder);
Operation operation = null;

// Create the builder separately from actually building, so we can note the channel that it created.
// (This is fairly unpleasant, but we'll try to improve this in the next version of GAX.)
Expand All @@ -258,11 +276,15 @@ private async Task<int> ExecuteDdlAsync(CancellationToken cancellationToken)
ProtoDescriptors = CommandTextBuilder.ProtobufDescriptors?.ToByteString() ?? ByteString.Empty,
};
var response = await databaseAdminClient.CreateDatabaseAsync(request).ConfigureAwait(false);
response = await response.PollUntilCompletedAsync().ConfigureAwait(false);
if (pollUntilCompleted)
Copy link
Contributor

@amanda-tarafa amanda-tarafa Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding this if in two places, let's have this method return the un-polled operation and the wrapping ones poll until completion if they need to.

Copy link
Contributor Author

@olavloite olavloite Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that really possible in this case, considering the fact that the two methods return two different operation types:

  1. Operation<Database, CreateDatabaseMetadata>
  2. Operation<Empty, UpdateDatabaseDdlMetadata>

(Type non-generic Operation class cannot be used for polling)

Copy link
Contributor

@robertvoinescu-work robertvoinescu-work Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the goal is to remove duplication might I instead suggest a a generic local function to centralize the polling and error-handling logic (at the bottom of this method):

async Task<Operation> HandleLro<TResp, TMeta>(Operation<TResp, TMeta> operation)
{
    if (pollUntilCompleted)
    {
        operation = await operation.PollUntilCompletedAsync().ConfigureAwait(false);
    }
    if (operation.IsFaulted)
    {
        throw SpannerException.FromOperationFailedException(operation.Exception);
    }
    return operation;
}

If we go the other way and use the returned Operation to poll later, the calling method will add extra overhead because it needs to establish a new channel since this method shuts down the one it creates in the finally block.

Copy link
Contributor

@amanda-tarafa amanda-tarafa Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes +1 to Robert's comment, I hadn't noticed the channel being closed.

{
response = await response.PollUntilCompletedAsync().ConfigureAwait(false);
}
if (response.IsFaulted)
{
throw SpannerException.FromOperationFailedException(response.Exception);
}
operation = response.RpcMessage;
}
else if (CommandTextBuilder.IsDropDatabaseCommand)
{
Expand Down Expand Up @@ -295,11 +317,15 @@ private async Task<int> ExecuteDdlAsync(CancellationToken cancellationToken)
};

var response = await databaseAdminClient.UpdateDatabaseDdlAsync(request).ConfigureAwait(false);
response = await response.PollUntilCompletedAsync().ConfigureAwait(false);
if (pollUntilCompleted)
{
response = await response.PollUntilCompletedAsync().ConfigureAwait(false);
}
if (response.IsFaulted)
{
throw SpannerException.FromOperationFailedException(response.Exception);
}
operation = response.RpcMessage;
}
}
catch (RpcException gRpcException)
Expand All @@ -312,7 +338,7 @@ private async Task<int> ExecuteDdlAsync(CancellationToken cancellationToken)
channel?.Shutdown();
}

return 0;
return operation;
}

private async Task<int> ExecuteMutationsAsync(CancellationToken cancellationToken)
Expand Down
16 changes: 16 additions & 0 deletions apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data/SpannerCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

using Google.Api.Gax;
using Google.Cloud.Spanner.V1;
using Google.LongRunning;
using System;
using System.Collections.Generic;
using System.Data;
Expand Down Expand Up @@ -513,6 +514,21 @@ public long ExecutePartitionedUpdate() =>
public Task<long> ExecutePartitionedUpdateAsync(CancellationToken cancellationToken = default) =>
CreateExecutableCommand().ExecutePartitionedUpdateAsync(cancellationToken);

/// <summary>
/// Executes this command as DDL, but does not wait for the execution of the DDL statements to finish. The
/// method returns the name of the long-running operation that was started. The cancellation token can only be
/// used to cancel the request to start the execution of the DDL statements. It cannot be used to cancel the
/// long-running operation once it has been started.
/// The command must contain one or more DDL statements;
/// <see cref="SpannerConnection.CreateDdlCommand(string, string[])"/> for details.
/// </summary>
/// <returns>
/// The name of the long-running operation that was started for the DDL statement(s).
/// Note: The ID is empty for DropDatabase commands.
/// </returns>
public Task<string> StartDdlAsync(CancellationToken cancellationToken = default) =>
CreateExecutableCommand().StartDdlAsync(cancellationToken);

/// <summary>
/// Creates an executable command that captures all the necessary information from this command.
/// </summary>
Expand Down