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.

Sign up for GitHub

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

Jump to bottom

CSV Uploads: catch failed connections and fix duplicate inserts #238

Merged
merged 2 commits into from
May 21, 2024
Merged

CSV Uploads: catch failed connections and fix duplicate inserts #238

merged 2 commits into from
May 21, 2024

Conversation

calherries
Copy link
Contributor

This PR fixes two issues:

  1. Fixes ClickHouse Cloud Uploads: Appending duplicate CSV file doesn't append new values #237. This is tested in the Metabase repo with a new test added by CSV uploads: Add test for appending duplicate data metabase/metabase#42918
  2. Fixes an issue introduced by CSV uploads for ClickHouse Cloud #236 where if the DB details don't lead to a successful connection, much of Metabase will be rendered completely unusable because of an uncaught exception from the new cloud? function.

calherries
calherries commented May 20, 2024
View reviewed changes
src/metabase/driver/clickhouse.clj
Comment on lines +188 to +189
;; disable insert idempotency to allow duplicate inserts
"SETTINGS replicated_deduplication_window = 0"]))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested by metabase/metabase#42918

@calherries calherries changed the title CSV Uploads: CSV Uploads: catch failed connections and fix duplicate inserts May 20, 2024
@slvrtrn
Copy link
Collaborator

slvrtrn commented May 21, 2024

@calherries, there is a CI error:

{:clojure.main/message
 "Syntax error (IllegalStateException) compiling clickhouse/cloud? at (metabase/driver/clickhouse_test.clj:106:44).\nvar: #'metabase.driver.clickhouse/cloud? is not public\n",
 :clojure.main/triage
 {:clojure.error/phase :compile-syntax-check,
  :clojure.error/line 106,
  :clojure.error/column 44,
  :clojure.error/source "clickhouse_test.clj",
  :clojure.error/symbol clickhouse/cloud?,
  :clojure.error/path "metabase/driver/clickhouse_test.clj",
  :clojure.error/class java.lang.IllegalStateException,
  :clojure.error/cause "var: #'metabase.driver.clickhouse/cloud? is not public"},
 :clojure.main/trace
 {:via
  [{:type clojure.lang.Compiler$CompilerException,
    :message "Syntax error compiling clickhouse/cloud? at (metabase/driver/clickhouse_test.clj:106:44).",
    :data
    {:clojure.error/phase :compile-syntax-check,
     :clojure.error/line 106,
     :clojure.error/column 44,
     :clojure.error/source "metabase/driver/clickhouse_test.clj",
     :clojure.error/symbol clickhouse/cloud?},
    :at [clojure.lang.Compiler analyzeSeq "Compiler.java" 7132]}
   {:type java.lang.IllegalStateException,
    :message "var: #'metabase.driver.clickhouse/cloud? is not public",
    :at [clojure.lang.Compiler isInline "Compiler.java" 6939]}],
  :trace
  [[clojure.lang.Compiler isInline "Compiler.java" 6939]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7117]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6135]
   [clojure.lang.Compiler$TryExpr$Parser parse "Compiler.java" 2326]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7124]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6137]
   [clojure.lang.Compiler$TryExpr$Parser parse "Compiler.java" 2326]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7124]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7112]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7112]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7112]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6137]
   [clojure.lang.Compiler$TryExpr$Parser parse "Compiler.java" 2326]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7124]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6137]
   [clojure.lang.Compiler$FnMethod parse "Compiler.java" 5479]
   [clojure.lang.Compiler$FnExpr parse "Compiler.java" 4041]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7122]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$InvokeExpr parse "Compiler.java" 3832]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7126]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$TryExpr$Parser parse "Compiler.java" 2297]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7124]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6135]
   [clojure.lang.Compiler$LetExpr$Parser parse "Compiler.java" 6453]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7124]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7112]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7112]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7112]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6135]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7124]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6137]
   [clojure.lang.Compiler$LetExpr$Parser parse "Compiler.java" 6453]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7124]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7112]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6137]
   [clojure.lang.Compiler$FnMethod parse "Compiler.java" 5479]
   [clojure.lang.Compiler$FnExpr parse "Compiler.java" 4041]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7122]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$InvokeExpr parse "Compiler.java" 3900]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7126]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7112]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6137]
   [clojure.lang.Compiler$FnMethod parse "Compiler.java" 5479]
   [clojure.lang.Compiler$FnExpr parse "Compiler.java" 4041]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7122]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$InvokeExpr parse "Compiler.java" 3900]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7126]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7112]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6137]
   [clojure.lang.Compiler$FnMethod parse "Compiler.java" 5479]
   [clojure.lang.Compiler$FnExpr parse "Compiler.java" 4041]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7122]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7112]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$MapExpr parse "Compiler.java" 3116]
   [clojure.lang.Compiler analyze "Compiler.java" 6814]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler$DefExpr$Parser parse "Compiler.java" 594]
   [clojure.lang.Compiler analyzeSeq "Compiler.java" 7124]
   [clojure.lang.Compiler analyze "Compiler.java" 6806]
   [clojure.lang.Compiler analyze "Compiler.java" 6762]
   [clojure.lang.Compiler eval "Compiler.java" 7198]
   [clojure.lang.Compiler load "Compiler.java" 7653]
   [clojure.lang.RT loadResourceScript "RT.java" 381]
   [clojure.lang.RT loadResourceScript "RT.java" 372]
   [clojure.lang.RT load "RT.java" 459]
   [clojure.lang.RT load "RT.java" 424]
   [clojure.core$load$fn__6908 invoke "core.clj" 6162]
   [clojure.core$load invokeStatic "core.clj" 6161]
   [clojure.core$load doInvoke "core.clj" 6145]
   [clojure.lang.RestFn invoke "RestFn.java" 408]
   [clojure.core$load_one invokeStatic "core.clj" 5934]
   [clojure.core$load_one invoke "core.clj" 5929]
   [clojure.core$load_lib$fn__6850 invoke "core.clj" 5976]
   [clojure.core$load_lib invokeStatic "core.clj" 5975]
   [clojure.core$load_lib doInvoke "core.clj" 5954]
   [clojure.lang.RestFn applyTo "RestFn.java" 142]
   [clojure.core$apply invokeStatic "core.clj" 669]
   [clojure.core$load_libs invokeStatic "core.clj" 6017]
   [clojure.core$load_libs doInvoke "core.clj" 6001]
   [clojure.lang.RestFn applyTo "RestFn.java" 137]
   [clojure.core$apply invokeStatic "core.clj" 669]
   [clojure.core$require invokeStatic "core.clj" 6039]
   [clojure.core$require doInvoke "core.clj" 6039]
   [clojure.lang.RestFn invoke "RestFn.java" 408]
   [mb.hawk.core$load_test_namespace invokeStatic "core.clj" 70]
   [mb.hawk.core$load_test_namespace invoke "core.clj" 68]
   [mb.hawk.core$find_tests_for_namespace_symbol invokeStatic "core.clj" 87]
   [mb.hawk.core$find_tests_for_namespace_symbol invoke "core.clj" 85]
   [mb.hawk.core$eval21935$fn__21936 invoke "core.clj" 102]
   [clojure.lang.MultiFn invoke "MultiFn.java" 234]
   [mb.hawk.core$find_tests_with_options invokeStatic "core.clj" 116]
   [mb.hawk.core$find_tests_with_options invoke "core.clj" 109]
   [mb.hawk.core$find_and_run_tests_with_options invokeStatic "core.clj" 214]
   [mb.hawk.core$find_and_run_tests_with_options invoke "core.clj" [20](https://github.com/ClickHouse/metabase-clickhouse-driver/actions/runs/9165895466/job/25226600702?pr=238#step:13:21)9]
   [mb.hawk.core$find_and_run_tests_cli invokeStatic "core.clj" 252]
   [mb.hawk.core$find_and_run_tests_cli invoke "core.clj" 244]
   [metabase.test_runner$find_and_run_tests_cli invokeStatic "test_runner.clj" 104]
   [metabase.test_runner$find_and_run_tests_cli invoke "test_runner.clj" 101]
   [clojure.lang.AFn applyToHelper "AFn.java" 154]
   [clojure.lang.AFn applyTo "AFn.java" 144]
   [clojure.lang.Var applyTo "Var.java" 705]
   [clojure.core$apply invokeStatic "core.clj" 667]
   [clojure.core$apply invoke "core.clj" 662]
   [clojure.run.exec$exec invokeStatic "exec.clj" 48]
   [clojure.run.exec$exec doInvoke "exec.clj" 39]
   [clojure.lang.RestFn invoke "RestFn.java" 4[23](https://github.com/ClickHouse/metabase-clickhouse-driver/actions/runs/9165895466/job/25226600702?pr=238#step:13:24)]
   [clojure.run.exec$_main$fn__20716 invoke "exec.clj" 180]
   [clojure.run.exec$_main invokeStatic "exec.clj" 176]
   [clojure.run.exec$_main doInvoke "exec.clj" 139]
   [clojure.lang.RestFn applyTo "RestFn.java" 137]
   [clojure.lang.Var applyTo "Var.java" 705]
   [clojure.core$apply invokeStatic "core.clj" 667]
   [clojure.main$main_opt invokeStatic "main.clj" 514]
   [clojure.main$main_opt invoke "main.clj" 510]
   [clojure.main$main invokeStatic "main.clj" 664]
   [clojure.main$main doInvoke "main.clj" 616]
   [clojure.lang.RestFn applyTo "RestFn.java" 137]
   [clojure.lang.Var applyTo "Var.java" 705]
   [clojure.main main "main.java" 40]],
  :cause "var: #'metabase.driver.clickhouse/cloud? is not public",
  :phase :compile-syntax-check}}

Syntax error (IllegalStateException) compiling clickhouse/cloud? at (metabase/driver/clickhouse_test.clj:106:44).
var: #'metabase.driver.clickhouse/cloud? is not public

@calherries
Copy link
Contributor Author

Thanks @slvrtrn, I've fixed it with fix reference to cloud?

@slvrtrn slvrtrn merged commit 47a64ec into ClickHouse:master May 21, 2024
1 check passed
@slvrtrn
Copy link
Collaborator

slvrtrn commented May 21, 2024

It seems like the CI is green, it just does not report it properly, so I am merging this. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slvrtrn slvrtrn slvrtrn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ClickHouse Cloud Uploads: Appending duplicate CSV file doesn't append new values
2 participants
@calherries @slvrtrn