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

Add NaN support #100

Merged
merged 6 commits into from
Jun 28, 2024
Merged

Add NaN support #100

merged 6 commits into from
Jun 28, 2024

Conversation

mkgrgis
Copy link
Contributor

@mkgrgis mkgrgis commented May 30, 2024

This implementation methodologically bases on

This PR implements NaN as readable value from SQLite if there is such case insensitive value with text affinity.

According ISO:SQL and implemented PostgreSQL code we can input NaN as special IEEE-754 value through API or as special text constant through SQL. SQLite doesn't support NaN with real affinity (in opposition to ±∞ values) and doesn't rewrites NULL value if NaN is detected. Hence in this PR NaN is readable and detectable but not writeable.

About changes:

  • NaN implementation was untested outside of aggregates test, 70f2f36 adds tests for previous implementation,
  • 026d6ef adds support for readable NaN value with text affinity in SQLite.

@mkgrgis
Copy link
Contributor Author

mkgrgis commented Jun 10, 2024

Hello, @t-kataym ! Which calendar plans have pgspider team to review this PR?

Copy link
Contributor

@MinhLA1410 MinhLA1410 left a comment

Choose a reason for hiding this comment

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

Hello @mkgrgis ,
Thank you for your support.
I have reviewed your PR. Could you please check it?

sqlite_query.c Outdated Show resolved Hide resolved
sqlite_query.c Outdated Show resolved Hide resolved
sql/16.0/extra/float8.sql Outdated Show resolved Hide resolved
@mkgrgis
Copy link
Contributor Author

mkgrgis commented Jun 17, 2024

I have reviewed your PR. Could you please check it?

Yes, @MinhLA1410 ! All of your comments are checked. All problems should be fixed in 9413bed. Please verify.

@mkgrgis mkgrgis requested a review from MinhLA1410 June 17, 2024 04:26
@mkgrgis
Copy link
Contributor Author

mkgrgis commented Jun 18, 2024

After merging of this PR #36 can be closed.

@mkgrgis
Copy link
Contributor Author

mkgrgis commented Jun 25, 2024

@MinhLA1410 , any updates?

@MinhLA1410
Copy link
Contributor

@mkgrgis Thank you. I have no futher comment.
I will leave the decision to @t-kataym

@t-kataym
Copy link
Contributor

@mkgrgis Thank you for your contribution. I will merge it.

@t-kataym t-kataym merged commit f2475fb into pgspider:master Jun 28, 2024
6 checks passed
@mkgrgis mkgrgis deleted the NaN branch June 28, 2024 09:15
mkgrgis added a commit to mkgrgis/sqlite_fdw that referenced this pull request Aug 8, 2024
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