Skip to content

feat: support for binary DOUBLE and ARRAY[DOUBLE] #38

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

Merged
merged 14 commits into from
Jul 14, 2025
Merged

feat: support for binary DOUBLE and ARRAY[DOUBLE] #38

merged 14 commits into from
Jul 14, 2025

Conversation

nwoolmer
Copy link
Contributor

@nwoolmer nwoolmer commented Jun 28, 2025

Upgrades the .NET client to for QuestDB OSS 9.0.0.

Changelist:

  • Refactor ISender into V1 and V2 versions.
  • Refactor Buffer into V1 and V2 versions, and add interface IBuffer.
  • Introduce array overloads to support sending multidimensional double arrays.
  • Introduce override to send doubles in binary format.
  • Introduce new protocol_version parameter, including automatic negotation if set to auto.
  • Add Nullable API to streamline sending nulls to QuestDB.

TODO in future PRs:

  • Support AoT compilation (compatibility fixes)
  • Introduce SenderPool which supports an API allowing you to send Buffers individually.

@nwoolmer nwoolmer self-assigned this Jun 28, 2025
@ideoma ideoma requested a review from Copilot July 11, 2025 09:45
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for binary DOUBLE and ARRAY[DOUBLE] in the .NET client and introduces protocol versioning (V1/V2) with auto-negotiation.

  • Refactor ISender and IBuffer into V1/V2 versions and add new overloads for multidimensional double arrays and binary doubles.
  • Add a protocol_version option (with an “auto” setting that negotiates on HTTP) and a factory method for creating the right buffer type.
  • Update TcpSender and HttpSender to use the new buffer factory and protocol V2 features.

Reviewed Changes

Copilot reviewed 50 out of 51 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/net-questdb-client/Senders/ISender.cs Split ISender into V1/V2 interfaces and added array/binary overloads
src/net-questdb-client/Utils/SenderOptions.cs Added protocol_version support and config-key validation set
src/net-questdb-client/Buffers/Buffer.cs Replaced Buffer implementation with a static factory
src/net-questdb-client/Buffers/BufferV2.cs Implemented V2-level binary DOUBLE and ARRAY support
src/net-questdb-client/Senders/HttpSender.cs Added HTTP negotiation for protocol version and use of new buffers
src/net-questdb-client/Senders/TcpSender.cs Updated to use new buffer factory and V2 features
Comments suppressed due to low confidence (5)

src/net-questdb-client/Utils/SenderOptions.cs:50

  • The field keySet should be PascalCase (KeySet) to match C# naming conventions for static fields.
    private static readonly HashSet<string> keySet = new()

src/net-questdb-client/Utils/SenderOptions.cs:72

  • Public properties should use PascalCase; consider renaming protocol_version to ProtocolVersion to follow C# conventions.
    private ProtocolVersion _protocol_version = ProtocolVersion.Auto;

src/net-questdb-client/Sender.cs:84

  • [nitpick] Throwing NotImplementedException for unsupported protocols can be confusing; consider using NotSupportedException to clearly indicate unsupported protocol values.
        throw new NotImplementedException();

src/net-questdb-client-tests/BufferTests.cs:3

  • This test class is empty; add unit tests for the new BufferV2 implementation and the Buffer.Create factory method to ensure binary and array support work as expected.
public class BufferTests

src/net-questdb-client/Utils/SettingsResponse.cs:111

  • [nitpick] The XML summary for the Preferences record is incomplete; consider expanding the description beyond "Contains" to explain what preferences are represented.
/// <summary>

Copy link
Collaborator

@ideoma ideoma left a comment

Choose a reason for hiding this comment

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

A few questions about the approach

Copy link
Collaborator

@ideoma ideoma left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

@ideoma ideoma merged commit 4784a7f into main Jul 14, 2025
1 check passed
@nwoolmer
Copy link
Contributor Author

Thanks for bringing this over the line @ideoma !

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.

2 participants