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: fix non-null constraint #13939

Merged
merged 15 commits into from
Dec 7, 2023

Conversation

JackTan25
Copy link
Contributor

@JackTan25 JackTan25 commented Dec 6, 2023

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

Summary about this PR

  1. for snowflake and postgres, it will return a non-null error when fill default value for non-null field.
  2. in this pr, when we try to fill default expr for a non-null field, if its default expr is none, we will return the error

This change is Reviewable

@github-actions github-actions bot added the pr-bugfix this PR patches a bug in codebase label Dec 6, 2023
@JackTan25 JackTan25 added the ci-cloud Build docker image for cloud test label Dec 6, 2023
Copy link
Contributor

github-actions bot commented Dec 6, 2023

Docker Image for PR

  • tag: pr-13939-db9f7ae

note: this image tag is only available for internal use,
please check the internal doc for more details.

@sundy-li
Copy link
Member

sundy-li commented Dec 7, 2023

This change is correct, but it is breaking changes. It may cause errors for old scripts or sink processes (insert by other drivers).
cc @BohuTANG @wubx

@JackTan25
Copy link
Contributor Author

JackTan25 commented Dec 7, 2023

when run script tests/suites/0_stateless/14_clickhouse_http_handler/14_0007_http_clickhouse_input_format_diagnostic.sh,we will
get a stack error, so the reason is below:

split_tx.send(Ok(Split {
    info: split_info,
    rx: data_rx,
}))
.await
.expect("fail to send split from stream"); // do expect here

let me remove the bad test added by me.

@JackTan25 JackTan25 force-pushed the fix_non_null_constraint branch from b7c458b to e3208fe Compare December 7, 2023 05:04
@JackTan25 JackTan25 added ci-cloud Build docker image for cloud test and removed ci-cloud Build docker image for cloud test labels Dec 7, 2023
Copy link
Contributor

github-actions bot commented Dec 7, 2023

Docker Image for PR

  • tag: pr-13939-86bc7f7

note: this image tag is only available for internal use,
please check the internal doc for more details.

@JackTan25 JackTan25 marked this pull request as ready for review December 7, 2023 06:05
@JackTan25 JackTan25 requested a review from sundy-li December 7, 2023 06:05
@JackTan25 JackTan25 requested a review from b41sh December 7, 2023 06:38
@JackTan25
Copy link
Contributor Author

pass checksb in distributed mode and standalone mode without A12 and A13

@JackTan25
Copy link
Contributor Author

JackTan25 commented Dec 7, 2023

for A12 and A13, we find out these in standalone and distributed mode.(snowflake will give an error in console, but in python script, it doesn't catch the error), these are expected.

-- MERGE-INTO-A12: transactions
MERGE INTO transactions USING (
    SELECT t.user_id, t.asset_type,
           SUM(t.quantity) AS total_quantity
    FROM transactions t
    GROUP BY t.user_id, t.asset_type
    UNION ALL
    SELECT t.user_id, 'NEW_TRANSACTION' AS asset_type,  -- New transaction type for insert
           30 AS quantity                                -- Example quantity
    FROM transactions t
    WHERE t.user_id % 2 = 0  -- Condition to select subset for new data
    GROUP BY t.user_id
) AS combined_transactions ON transactions.user_id = combined_transactions.user_id AND transactions.asset_type = combined_transactions.asset_type
    WHEN MATCHED THEN
        UPDATE SET transactions.quantity = combined_transactions.total_quantity
    WHEN NOT MATCHED THEN
        INSERT (user_id, transaction_type, asset_type, quantity, transaction_time)
            VALUES (combined_transactions.user_id, 'trade', combined_transactions.asset_type, combined_transactions.total_quantity, '2023-01-01') -D mergeinto
bendsql command failed: warning: --database is ignored when --dsn is set
Error: APIError: ResponseError with 1006: null value in column `transaction_id` violates not-null constraint

Traceback (most recent call last):
  File "/home/ubuntu/github/wizard2/checksb/checksb.py", line 66, in execute_sql
    result = subprocess.run(command, text=True, capture_output=True, check=True)
  File "/usr/lib/python3.10/subprocess.py", line 526, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['bendsql', "--query=\n\n-- MERGE-INTO-A12: transactions\nMERGE INTO transactions USING (\n    SELECT t.user_id, t.asset_type,\n           SUM(t.quantity) AS total_quantity\n    FROM transactions t\n    GROUP BY t.user_id, t.asset_type\n    UNION ALL\n    SELECT t.user_id, 'NEW_TRANSACTION' AS asset_type,  -- New transaction type for insert\n           30 AS quantity                                -- Example quantity\n    FROM transactions t\n    WHERE t.user_id % 2 = 0  -- Condition to select subset for new data\n    GROUP BY t.user_id\n) AS combined_transactions ON transactions.user_id = combined_transactions.user_id AND transactions.asset_type = combined_transactions.asset_type\n    WHEN MATCHED THEN\n        UPDATE SET transactions.quantity = combined_transactions.total_quantity\n    WHEN NOT MATCHED THEN\n        INSERT (user_id, transaction_type, asset_type, quantity, transaction_time)\n            VALUES (combined_transactions.user_id, 'trade', combined_transactions.asset_type, combined_transactions.total_quantity, '2023-01-01')", '-D', 'mergeinto']' returned non-zero exit status 1.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/ubuntu/github/wizard2/checksb/checksb.py", line 165, in <module>
    main()
  File "/home/ubuntu/github/wizard2/checksb/checksb.py", line 151, in main
    execute_sql_scripts("bendsql", f"{base_sql_dir}/action.sql", database_name)
  File "/home/ubuntu/github/wizard2/checksb/checksb.py", line 83, in execute_sql_scripts
    execute_sql(query, sql_tool, database, warehouse)
  File "/home/ubuntu/github/wizard2/checksb/checksb.py", line 73, in execute_sql
    raise RuntimeError(error_message)
RuntimeError: bendsql command failed: warning: --database is ignored when --dsn is set
Error: APIError: ResponseError with 1006: null value in column `transaction_id` violates not-null constraint

@JackTan25
Copy link
Contributor Author

cc @BohuTANG merge into wizard test is passed, can we merge this?

@BohuTANG
Copy link
Member

BohuTANG commented Dec 7, 2023

This change is correct, but it is breaking changes. It may cause errors for old scripts or sink processes (insert by other drivers). cc @BohuTANG @wubx

Worth to do, we can highlight this in the changelog and our doc, tell the user to add the default value for the not null column if they are not defined before.

@BohuTANG
Copy link
Member

BohuTANG commented Dec 7, 2023

A12/A13 need be fixed to make them works for Databend and Snowflake, I will do it this weekend. @JackTan25

@BohuTANG BohuTANG merged commit ae4da22 into databendlabs:main Dec 7, 2023
68 checks passed
@BohuTANG
Copy link
Member

BohuTANG commented Dec 8, 2023

A12/A13 need be fixed to make them works for Databend and Snowflake, I will do it this weekend. @JackTan25

Fixed: databendlabs/wizard@9cd4d96

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-cloud Build docker image for cloud test pr-bugfix this PR patches a bug in codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants