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

fix(binder): insert column count validation #111

Merged
merged 2 commits into from
Dec 20, 2023

Conversation

Joeyscat
Copy link
Contributor

@Joeyscat Joeyscat commented Dec 18, 2023

What problem does this PR solve?

Add corresponding issue link with summary if exists -->

Issue link:
#110

What is changed and how it works?

Code changes

  • Has Rust code change
  • Has CI related scripts change

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Note for reviewer

@KKould
Copy link
Member

KKould commented Dec 19, 2023

It looks like omitting the padding of the default values ​​is causing the e2e test to fail

@KKould KKould self-requested a review December 19, 2023 15:52
@KKould KKould added the bug Something isn't working label Dec 19, 2023
When column names are not specified, the length of values ​​cannot exceed the number of columns of the table.
When column names are specified, values ​​need to be the same length to given column names.
@Joeyscat
Copy link
Contributor Author

It looks like omitting the padding of the default values ​​is causing the e2e test to fail

😀Fixed, e2e tests now pass.

@KKould
Copy link
Member

KKould commented Dec 20, 2023

Everything looks good, thank you very much for your contribution

@KKould KKould merged commit 585dbeb into KipData:main Dec 20, 2023
3 checks passed
@KKould KKould added invalid This doesn't seem right and removed bug Something isn't working labels Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants