Skip to content

Commit 1f93984

Browse files
hafenkranhafenkran
authored
Merge pull request #131 from hafenkran/format_tidy_cleanup
Added code-quality-check to CI
2 parents 82310dd + 32c307d commit 1f93984

File tree

83 files changed

+354
-933
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

83 files changed

+354
-933
lines changed

‎.github/workflows/MainDistributionPipeline.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,15 @@ concurrency:
1212
cancel-in-progress: true
1313

1414
jobs:
15+
code-quality-check:
16+
name: Code Quality Check
17+
uses: duckdb/extension-ci-tools/.github/workflows/[email protected]
18+
with:
19+
duckdb_version: v1.4.1
20+
ci_tools_version: v1.4.1
21+
extension_name: bigquery
22+
format_checks: "format"
23+
1524
duckdb-stable-build:
1625
name: Build extension binaries
1726
if: |

‎src/bigquery_arrow_scan.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ unique_ptr<FunctionData> BigqueryArrowScanFunction::BigqueryArrowScanBind(Client
3131

3232
auto table_string = input.inputs[0].GetValue<string>();
3333
auto table_ref = BigqueryUtils::ParseTableString(table_string);
34-
if (!table_ref.has_dataset_id() || !table_ref.has_table_id()) {
34+
if (!table_ref.HasDatasetId() || !table_ref.HasTableId()) {
3535
throw BinderException("Invalid table string: %s", table_string);
3636
}
3737

@@ -129,7 +129,11 @@ unique_ptr<GlobalTableFunctionState> BigqueryArrowScanFunction::BigqueryArrowSca
129129

130130
// Wrap reader in a factory so every thread can open its own stream
131131
auto factory = make_shared_ptr<BigqueryStreamFactory>(bq_arrow_reader);
132-
bind_data.factory_dep()->factory = factory;
132+
auto factory_dependency = bind_data.GetFactoryDependency();
133+
if (!factory_dependency) {
134+
throw InternalException("Factory dependency not initialized");
135+
}
136+
factory_dependency->factory = factory;
133137
bind_data.stream_factory_ptr = reinterpret_cast<uintptr_t>(factory.get());
134138

135139
// Initialize global scan state

‎src/bigquery_client.cpp‎

Lines changed: 50 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,15 @@ google::cloud::Options BigqueryClient::OptionsAPI() {
112112
}
113113

114114
bool credentials_set = false;
115-
auto secret_match = LookupBigQuerySecret(*context, config.project_id);
116-
if (secret_match.HasMatch()) {
117-
auto &bq_secret = dynamic_cast<const BigquerySecret &>(secret_match.GetSecret());
118-
auto credentials = CreateGCPCredentialsFromSecret(bq_secret);
119-
if (credentials) {
120-
options.set<google::cloud::v2_38::UnifiedCredentialsOption>(credentials);
121-
credentials_set = true;
122-
}
123-
}
115+
auto secret_match = LookupBigQuerySecret(*context, config.project_id);
116+
if (secret_match.HasMatch()) {
117+
auto &bq_secret = dynamic_cast<const BigquerySecret &>(secret_match.GetSecret());
118+
auto credentials = CreateGCPCredentialsFromSecret(bq_secret);
119+
if (credentials) {
120+
options.set<google::cloud::v2_38::UnifiedCredentialsOption>(credentials);
121+
credentials_set = true;
122+
}
123+
}
124124

125125
// Set CA bundle path if provided
126126
auto ca_path = BigquerySettings::CurlCaBundlePath();
@@ -147,15 +147,15 @@ google::cloud::Options BigqueryClient::OptionsGRPC() {
147147
}
148148

149149
bool credentials_set = false;
150-
auto secret_match = LookupBigQuerySecret(*context, config.project_id);
151-
if (secret_match.HasMatch()) {
152-
auto &bq_secret = dynamic_cast<const BigquerySecret &>(secret_match.GetSecret());
153-
auto credentials = CreateGCPCredentialsFromSecret(bq_secret);
154-
if (credentials) {
155-
options.set<google::cloud::v2_38::UnifiedCredentialsOption>(credentials);
156-
credentials_set = true;
157-
}
158-
}
150+
auto secret_match = LookupBigQuerySecret(*context, config.project_id);
151+
if (secret_match.HasMatch()) {
152+
auto &bq_secret = dynamic_cast<const BigquerySecret &>(secret_match.GetSecret());
153+
auto credentials = CreateGCPCredentialsFromSecret(bq_secret);
154+
if (credentials) {
155+
options.set<google::cloud::v2_38::UnifiedCredentialsOption>(credentials);
156+
credentials_set = true;
157+
}
158+
}
159159

160160
// Set default credentials if no secret credentials were set
161161
if (!credentials_set) {
@@ -924,46 +924,46 @@ void BigqueryClient::MapInformationSchemaRows(
924924
}
925925

926926
void BigqueryClient::CheckAuthentication() {
927-
if (authentication_checked) {
928-
return;
929-
}
927+
if (authentication_checked) {
928+
return;
929+
}
930930

931-
// Check if we have a DuckDB secret for this project
932-
auto secret_match = LookupBigQuerySecret(*context, config.project_id);
933-
if (secret_match.HasMatch()) {
934-
authentication_checked = true;
935-
return; // If we have a secret, we assume being able to authenticate
936-
}
931+
// Check if we have a DuckDB secret for this project
932+
auto secret_match = LookupBigQuerySecret(*context, config.project_id);
933+
if (secret_match.HasMatch()) {
934+
authentication_checked = true;
935+
return; // If we have a secret, we assume being able to authenticate
936+
}
937937

938938
// Check if ADC environment variables are set
939-
auto adc_credential = google::cloud::oauth2_internal::GoogleAdcFilePathFromEnvVarOrEmpty();
940-
if (!adc_credential.empty()) {
941-
std::ifstream creds_file(adc_credential);
942-
if (creds_file.good()) {
943-
authentication_checked = true;
944-
return; // ADC file exists
945-
}
946-
}
947-
948-
// Check if gcloud CLI credentials exist
949-
auto gcloud_creds_path = google::cloud::oauth2_internal::GoogleAdcFilePathFromWellKnownPathOrEmpty();
950-
if (!gcloud_creds_path.empty()) {
951-
std::ifstream creds_file(gcloud_creds_path);
952-
if (creds_file.good()) {
953-
authentication_checked = true;
954-
return; // gcloud ADC file exists
955-
}
956-
}
957-
958-
// Check if we're in a GCP environment (i.e., metadata server set)
939+
auto adc_credential = google::cloud::oauth2_internal::GoogleAdcFilePathFromEnvVarOrEmpty();
940+
if (!adc_credential.empty()) {
941+
std::ifstream creds_file(adc_credential);
942+
if (creds_file.good()) {
943+
authentication_checked = true;
944+
return; // ADC file exists
945+
}
946+
}
947+
948+
// Check if gcloud CLI credentials exist
949+
auto gcloud_creds_path = google::cloud::oauth2_internal::GoogleAdcFilePathFromWellKnownPathOrEmpty();
950+
if (!gcloud_creds_path.empty()) {
951+
std::ifstream creds_file(gcloud_creds_path);
952+
if (creds_file.good()) {
953+
authentication_checked = true;
954+
return; // gcloud ADC file exists
955+
}
956+
}
957+
958+
// Check if we're in a GCP environment (i.e., metadata server set)
959959
const char *gcp_project = std::getenv("GOOGLE_CLOUD_PROJECT");
960960
const char *gce_metadata = std::getenv("GCE_METADATA_ROOT");
961961
if (gcp_project || gce_metadata) {
962-
authentication_checked = true;
962+
authentication_checked = true;
963963
return; // Likely running on GCP, should have automatic credentials
964964
}
965965

966-
// No authentication method found
966+
// No authentication method found
967967
throw InvalidInputException(
968968
"BigQuery Authentication Failed\n"
969969
"\n"
@@ -1005,7 +1005,7 @@ void BigqueryClient::ThrowOnErrorStatus(const google::cloud::Status &status) {
10051005
"Your credentials are valid, but lack the necessary permissions for this operation.\n"
10061006
"\n"
10071007
"Required actions:\n"
1008-
" 1. Verify you are using the correct credentials for this project"
1008+
" 1. Verify you are using the correct credentials for this project"
10091009
" 2. Verify your credentials have the required BigQuery roles:\n"
10101010
" • BigQuery Data Viewer (roles/bigquery.dataViewer) - for read access\n"
10111011
" • BigQuery Data Editor (roles/bigquery.dataEditor) - for write access\n"

‎src/bigquery_execute.cpp‎

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ struct BigQueryExecuteBindData : public TableFunctionData {
1414
BigqueryConfig config;
1515
shared_ptr<BigqueryClient> bq_client;
1616
string query;
17-
bool dry_run = false;
17+
bool dry_run = false;
1818
bool finished = false;
1919
};
2020

@@ -30,7 +30,7 @@ static duckdb::unique_ptr<FunctionData> BigQueryExecuteBind(ClientContext &conte
3030

3131
auto result = make_uniq<BigQueryExecuteBindData>();
3232
result->query = query_string;
33-
result->dry_run = params.dry_run;
33+
result->dry_run = params.dry_run;
3434

3535
auto &database_manager = DatabaseManager::Get(context);
3636
auto database = database_manager.GetDatabase(context, dbname_or_project_id);
@@ -60,29 +60,29 @@ static duckdb::unique_ptr<FunctionData> BigQueryExecuteBind(ClientContext &conte
6060
result->bq_client = make_shared_ptr<BigqueryClient>(context, result->config);
6161
}
6262

63-
if (!params.dry_run) {
64-
return_types.emplace_back(LogicalTypeId::BOOLEAN);
65-
names.emplace_back("success");
66-
return_types.emplace_back(LogicalTypeId::VARCHAR);
67-
names.emplace_back("job_id");
68-
return_types.emplace_back(LogicalTypeId::VARCHAR);
69-
names.emplace_back("project_id");
70-
return_types.emplace_back(LogicalTypeId::VARCHAR);
71-
names.emplace_back("location");
72-
return_types.emplace_back(LogicalTypeId::UBIGINT);
73-
names.emplace_back("total_rows");
74-
return_types.emplace_back(LogicalTypeId::BIGINT);
75-
names.emplace_back("total_bytes_processed");
76-
return_types.emplace_back(LogicalTypeId::VARCHAR);
77-
names.emplace_back("num_dml_affected_rows");
78-
} else {
79-
return_types.emplace_back(LogicalTypeId::BIGINT);
80-
names.emplace_back("total_bytes_processed");
81-
return_types.emplace_back(LogicalTypeId::BOOLEAN);
82-
names.emplace_back("cache_hit");
83-
return_types.emplace_back(LogicalTypeId::VARCHAR);
84-
names.emplace_back("location");
85-
}
63+
if (!params.dry_run) {
64+
return_types.emplace_back(LogicalTypeId::BOOLEAN);
65+
names.emplace_back("success");
66+
return_types.emplace_back(LogicalTypeId::VARCHAR);
67+
names.emplace_back("job_id");
68+
return_types.emplace_back(LogicalTypeId::VARCHAR);
69+
names.emplace_back("project_id");
70+
return_types.emplace_back(LogicalTypeId::VARCHAR);
71+
names.emplace_back("location");
72+
return_types.emplace_back(LogicalTypeId::UBIGINT);
73+
names.emplace_back("total_rows");
74+
return_types.emplace_back(LogicalTypeId::BIGINT);
75+
names.emplace_back("total_bytes_processed");
76+
return_types.emplace_back(LogicalTypeId::VARCHAR);
77+
names.emplace_back("num_dml_affected_rows");
78+
} else {
79+
return_types.emplace_back(LogicalTypeId::BIGINT);
80+
names.emplace_back("total_bytes_processed");
81+
return_types.emplace_back(LogicalTypeId::BOOLEAN);
82+
names.emplace_back("cache_hit");
83+
return_types.emplace_back(LogicalTypeId::VARCHAR);
84+
names.emplace_back("location");
85+
}
8686

8787
return result;
8888
}
@@ -95,19 +95,19 @@ static void BigQueryExecuteFunc(ClientContext &context, TableFunctionInput &data
9595
auto response = data.bq_client->ExecuteQuery(data.query, "", data.dry_run);
9696
data.finished = true;
9797

98-
if (!data.dry_run) {
99-
output.SetValue(0, 0, true);
100-
output.SetValue(1, 0, response.job_reference().job_id());
101-
output.SetValue(2, 0, response.job_reference().project_id());
102-
output.SetValue(3, 0, response.job_reference().location().value());
103-
output.SetValue(4, 0, Value::UBIGINT(response.total_rows().value()));
104-
output.SetValue(5, 0, Value::BIGINT(response.total_bytes_processed().value()));
105-
output.SetValue(6, 0, Value::BIGINT(response.num_dml_affected_rows().value()));
106-
} else {
107-
output.SetValue(0, 0, Value::BIGINT(response.total_bytes_processed().value()));
108-
output.SetValue(1, 0, Value::BOOLEAN(response.cache_hit().value()));
109-
output.SetValue(2, 0, response.job_reference().location().value());
110-
}
98+
if (!data.dry_run) {
99+
output.SetValue(0, 0, true);
100+
output.SetValue(1, 0, response.job_reference().job_id());
101+
output.SetValue(2, 0, response.job_reference().project_id());
102+
output.SetValue(3, 0, response.job_reference().location().value());
103+
output.SetValue(4, 0, Value::UBIGINT(response.total_rows().value()));
104+
output.SetValue(5, 0, Value::BIGINT(response.total_bytes_processed().value()));
105+
output.SetValue(6, 0, Value::BIGINT(response.num_dml_affected_rows().value()));
106+
} else {
107+
output.SetValue(0, 0, Value::BIGINT(response.total_bytes_processed().value()));
108+
output.SetValue(1, 0, Value::BOOLEAN(response.cache_hit().value()));
109+
output.SetValue(2, 0, response.job_reference().location().value());
110+
}
111111
output.SetCardinality(1);
112112
}
113113

@@ -119,7 +119,7 @@ BigQueryExecuteFunction::BigQueryExecuteFunction()
119119

120120
named_parameters["api_endpoint"] = LogicalType::VARCHAR;
121121
named_parameters["grpc_endpoint"] = LogicalType::VARCHAR;
122-
named_parameters["dry_run"] = LogicalType::BOOLEAN;
122+
named_parameters["dry_run"] = LogicalType::BOOLEAN;
123123
}
124124

125125
} // namespace bigquery

‎src/bigquery_extension.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ static void LoadInternal(ExtensionLoader &loader) {
5656
auto &config = DBConfig::GetConfig(loader.GetDatabaseInstance());
5757
config.storage_extensions["bigquery"] = make_uniq<bigquery::BigqueryStorageExtension>();
5858

59-
bigquery::RegisterBigquerySecretType(loader.GetDatabaseInstance());
59+
bigquery::RegisterBigquerySecretType(loader.GetDatabaseInstance());
6060

6161
// Register WKT->GEOMETRY cast (runtime lookup of spatial extension's ST_GeomFromText)
6262
bigquery::RegisterWKTGeometryCast(loader.GetDatabaseInstance());

‎src/bigquery_info.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ namespace bigquery {
88
BigqueryCreateSchemaInfo::BigqueryCreateSchemaInfo() : CreateSchemaInfo() {
99
}
1010

11-
BigqueryCreateSchemaInfo::BigqueryCreateSchemaInfo(const CreateSchemaInfo &info,
12-
unordered_map<string, string> options)
11+
BigqueryCreateSchemaInfo::BigqueryCreateSchemaInfo(const CreateSchemaInfo &info, unordered_map<string, string> options)
1312
: CreateSchemaInfo(), options(std::move(options)) {
1413
this->info_type = info.info_type;
1514
this->catalog = info.catalog;
@@ -58,8 +57,7 @@ BigqueryCreateTableInfo::BigqueryCreateTableInfo(string catalog,
5857
: CreateTableInfo(std::move(catalog), std::move(schema), std::move(name)), options(std::move(options)) {
5958
}
6059

61-
BigqueryCreateTableInfo::BigqueryCreateTableInfo(const CreateTableInfo &info,
62-
unordered_map<string, string> options)
60+
BigqueryCreateTableInfo::BigqueryCreateTableInfo(const CreateTableInfo &info, unordered_map<string, string> options)
6361
: CreateTableInfo(info.catalog, info.schema, info.table), options(std::move(options)) {
6462
this->info_type = info.info_type;
6563
this->type = info.type;

0 commit comments

Comments
 (0)