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

[C-API] Data chunk invalid left-shift #4660

Merged
merged 20 commits into from
Sep 12, 2022

Conversation

Tishj
Copy link
Contributor

@Tishj Tishj commented Sep 9, 2022

This PR fixes #4499

It's a simple oversight, the type of 1 defaults to int32_t, causing a shift with any value larger than/equal to 32 produces an incorrect result.

@hannes
Copy link
Member

hannes commented Sep 10, 2022

As suggested in the issue, can you please double check whether there are other problematic instances of this shift in the codebase?

@Tishj
Copy link
Contributor Author

Tishj commented Sep 10, 2022

As suggested in the issue, can you please double check whether there are other problematic instances of this shift in the codebase?

Yes will do 👍

@hannes
Copy link
Member

hannes commented Sep 10, 2022

Thanks can you also double check what happened to your ICU? There's a giant diff there.

@Tishj
Copy link
Contributor Author

Tishj commented Sep 10, 2022

Yea it's caused by format-master, I'll redo the changes without formatting I guess

I somehow broke Julia, it cant find duckdb_pending_prepared anymore

@hannes
Copy link
Member

hannes commented Sep 11, 2022

Thanks, but I would probably say let's not do this fix in the third_party stuff. @Mytherin do you agree?

@Mytherin
Copy link
Collaborator

Agreed let’s skip the ICU fixes, it’s likely the code is never even run

@hannes hannes merged commit 794992a into duckdb:master Sep 12, 2022
begelundmuller added a commit to begelundmuller/go-duckdb that referenced this pull request Sep 12, 2022
marcboeker pushed a commit to marcboeker/go-duckdb that referenced this pull request Sep 27, 2022
* Use DuckDB amalgamation instead of shared libraries

* Patch amalgamation for Windows pending release of duckdb/duckdb@fe328d3

* Add build flags for Windows compilation

* Bundle static libraries, and make source and shared libraries optional

* Run go test only in root

* Prepare deps workflow for merge into main

* Fix include path for compile from source

* Fix bug for compiling from source

* Trigger deps build when DUCKDB_VERSION is updated

* Test automated deps PR by downgrading to v0.4.0

* Test automated deps PR by downgrading to v0.4.0

* CI to build static libraries when upgrading DuckDB version

* Fix bug in deps.yaml

* Change use_source to from_soure

* Fix bug in static library commit

* Copy over fix from duckdb/duckdb#4660

* Test with touch

* Test on all paths

* Bugfix: bad path name in CI

* Re-build static libraries

* Now run actual build

* Re-build static libraries

* Update documentation

* Clean up deps build

* Tidy up imports

* bugfix: empty results would return a row

* Clean up gcc flags

* Run lib builds (tmp)

* Trigger build on main

* Re-build static libraries

* Revert to build static libs only on PR

* Build static libs on non-master changes to duckdb files

Co-authored-by: begelundmuller <begelundmuller@users.noreply.github.com>
@Tishj Tishj deleted the capi_datachunk_dflt-value_bug branch October 5, 2022 07:28
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.

PRAGMA table_info returns invalid dflt_value using data chunks API
3 participants