Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix the NpgsqlCube to use the full G17 floating-point format - #6295

#6295
Merged
roji merged 2 commits into
npgsql:mainnpgsql/npgsql:mainfrom
kirkbrauer:fix-cube-writekirkbrauer/npgsql:fix-cube-writeCopy head branch name to clipboard
Nov 7, 2025
Merged

Fix the NpgsqlCube to use the full G17 floating-point format#6295
roji merged 2 commits into
npgsql:mainnpgsql/npgsql:mainfrom
kirkbrauer:fix-cube-writekirkbrauer/npgsql:fix-cube-writeCopy head branch name to clipboard

Conversation

@kirkbrauer

Copy link
Copy Markdown
Contributor
  • Switch cube floating-point string to use G17 format for 17 digits of precision to follow IEEE 754

@kirkbrauer

kirkbrauer commented Nov 6, 2025

Copy link
Copy Markdown
Contributor Author

@roji one question I did have is if we should remove the redundant ToSubset() method in the NpgsqlCube now that we have the Subset() extension method. It might be confusing to users to have both, but only one can be translated. A good intermediate could be to just rename it to Subset() which would violate the naming convention, but at least it would be consistent and not introduce another method.

@NinoFloris any thoughts on this?

@kirkbrauer kirkbrauer changed the title Fix the Write() method to use the full G17 floating-point format Fix the NpgsqlCube to use the full G17 floating-point format Nov 6, 2025

@roji roji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

one question I did have is if we should remove the redundant ToSubset() method in the NpgsqlCube now that we have the Subset() extension method.

I don't think the API of NpgsqlCube should be changed as a function of what we're doing in the EF provider - there may be Npgsql users which don't use EF and want to interact with Cube. So if we think that a client-side implementation of Subset makes sense to have, we should keep it in Npgsql.

Besides, given the conclusion we seem to have gotten to in npgsql/efcore.pg#3651 (comment)), do we even need an extension method in the EF provider? Why not just translate the ToSubset method on NpgsqlCube?

Unrelated: I noticed that NpgsqlCube.cs uses a namespace block instead of a statement, can you please change that in this PR as well to clean up the code? All of Npgsql uses namespace statements.

@kirkbrauer

Copy link
Copy Markdown
Contributor Author

@roji Ok, I think we will unify on the ToSubset() method instead and just translate it when we find it in efcore.pg and remove the extension method.

@roji
roji merged commit 33d8cdc into npgsql:main Nov 7, 2025
13 checks passed
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.