Skip to content

Commit 5e69a25

Browse files
author
Michael Harris
committed
chore(copy): refactor option value extraction
1 parent f2d58b5 commit 5e69a25

1 file changed

Lines changed: 50 additions & 117 deletions

File tree

‎src/gsheets_copy.cpp‎

Lines changed: 50 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
#include <vector>
1+
#include <utility>
22

3+
#include "duckdb/common/case_insensitive_map.hpp"
34
#include "duckdb/common/exception.hpp"
45
#include "duckdb/common/exception/binder_exception.hpp"
56
#include "duckdb/common/file_system.hpp"
7+
#include "duckdb/common/types.hpp"
68
#include "duckdb/common/types/value.hpp"
79

810
#include "gsheets_copy.hpp"
@@ -24,131 +26,62 @@ GSheetCopyFunction::GSheetCopyFunction() : CopyFunction("gsheet") {
2426
copy_to_sink = GSheetWriteSink;
2527
}
2628

29+
static std::string GetStringOption(const case_insensitive_map_t<vector<Value>> &options, const std::string &name,
30+
const std::string &default_value = "") {
31+
const auto it = options.find(name);
32+
if (it == options.end()) {
33+
return default_value;
34+
}
35+
std::string err;
36+
Value val;
37+
if (!it->second.back().DefaultTryCastAs(LogicalType::VARCHAR, val, &err)) {
38+
throw BinderException(name + " option must be VARCHAR");
39+
}
40+
if (val.IsNull()) {
41+
throw BinderException(name + " option must not be NULL");
42+
}
43+
return StringValue::Get(val);
44+
}
45+
46+
// NOTE: the second value in pair is a flag indicating if the value was set by the user
47+
static std::pair<bool, bool> GetBoolOption(const case_insensitive_map_t<vector<Value>> &options,
48+
const std::string &name, bool default_value = false) {
49+
const auto it = options.find(name);
50+
if (it == options.end()) {
51+
return std::make_pair(default_value, false);
52+
}
53+
if (it->second.size() != 1) {
54+
throw BinderException(name + " option must be a single boolean value");
55+
}
56+
std::string err;
57+
Value val;
58+
if (!it->second.back().DefaultTryCastAs(LogicalType::BOOLEAN, val, &err)) {
59+
throw BinderException(name + " option must be a single boolean value");
60+
}
61+
if (val.IsNull()) {
62+
throw BinderException(name + " option must be a single boolean value");
63+
}
64+
return std::make_pair(BooleanValue::Get(val), true);
65+
}
66+
2767
unique_ptr<FunctionData> GSheetCopyFunction::GSheetWriteBind(ClientContext &context, CopyFunctionBindInput &input,
2868
const vector<string> &names,
2969
const vector<LogicalType> &sql_types) {
3070

31-
// TODO(mh): refactor option resolution
3271
string file_path = input.info.file_path;
3372
auto options = input.info.options;
3473

35-
const auto sheet_opt = options.find("sheet");
36-
std::string sheet;
37-
if (sheet_opt != options.end()) {
38-
string error_msg;
39-
Value sheet_val;
40-
if (!sheet_opt->second.back().DefaultTryCastAs(LogicalType::VARCHAR, sheet_val, &error_msg)) {
41-
throw BinderException("sheet option must be a VARCHAR");
42-
}
43-
if (sheet_val.IsNull()) {
44-
throw BinderException("sheet option must be a non-null VARCHAR");
45-
}
46-
sheet = StringValue::Get(sheet_val);
47-
} else {
48-
sheet = "";
49-
}
50-
51-
const auto range_opt = options.find("range");
52-
std::string range;
53-
if (range_opt != options.end()) {
54-
string error_msg;
55-
Value range_val;
56-
if (!range_opt->second.back().DefaultTryCastAs(LogicalType::VARCHAR, range_val, &error_msg)) {
57-
throw BinderException("range option must be a VARCHAR");
58-
}
59-
if (range_val.IsNull()) {
60-
throw BinderException("range option must be a non-null VARCHAR");
61-
}
62-
range = StringValue::Get(range_val);
63-
} else {
64-
range = "";
65-
}
66-
67-
const auto overwrite_sheet_opt = options.find("overwrite_sheet");
68-
bool overwrite_sheet;
69-
if (overwrite_sheet_opt != options.end()) {
70-
if (overwrite_sheet_opt->second.size() != 1) {
71-
throw BinderException("overwrite_sheet option must be a single boolean value");
72-
}
73-
string error_msg;
74-
Value overwrite_sheet_bool_val;
75-
if (!overwrite_sheet_opt->second.back().DefaultTryCastAs(LogicalType::BOOLEAN, overwrite_sheet_bool_val,
76-
&error_msg)) {
77-
throw BinderException("overwrite_sheet option must be a single boolean value");
78-
}
79-
if (overwrite_sheet_bool_val.IsNull()) {
80-
throw BinderException("overwrite_sheet option must be a single boolean value");
81-
}
82-
overwrite_sheet = BooleanValue::Get(overwrite_sheet_bool_val);
83-
} else {
84-
overwrite_sheet = true; // Default to overwrite_sheet to maintain prior behavior
85-
}
86-
87-
const auto overwrite_range_opt = options.find("overwrite_range");
88-
bool overwrite_range;
89-
if (overwrite_range_opt != options.end()) {
90-
if (overwrite_range_opt->second.size() != 1) {
91-
throw BinderException("overwrite_range option must be a single boolean value");
92-
}
93-
string error_msg;
94-
Value overwrite_range_bool_val;
95-
if (!overwrite_range_opt->second.back().DefaultTryCastAs(LogicalType::BOOLEAN, overwrite_range_bool_val,
96-
&error_msg)) {
97-
throw BinderException("overwrite_range option must be a single boolean value");
98-
}
99-
if (overwrite_range_bool_val.IsNull()) {
100-
throw BinderException("overwrite_range option must be a single boolean value");
101-
}
102-
overwrite_range = BooleanValue::Get(overwrite_range_bool_val);
103-
} else {
104-
overwrite_range = false;
105-
}
106-
107-
const auto header_opt = options.find("header");
108-
bool header;
109-
if (header_opt != options.end()) {
110-
if (header_opt->second.size() != 1) {
111-
throw BinderException("header option must be a single boolean value");
112-
}
113-
string error_msg;
114-
Value header_bool_val;
115-
if (!header_opt->second.back().DefaultTryCastAs(LogicalType::BOOLEAN, header_bool_val, &error_msg)) {
116-
throw BinderException("header option must be a single boolean value");
117-
}
118-
if (header_bool_val.IsNull()) {
119-
throw BinderException("header option must be a single boolean value");
120-
}
121-
header = BooleanValue::Get(header_bool_val);
122-
} else {
123-
header = true;
124-
// If we are in the append case, default to no header instead.
125-
if (!overwrite_sheet && !overwrite_range) {
126-
header = false;
127-
}
128-
}
74+
auto sheet = GetStringOption(options, "sheet");
75+
auto range = GetStringOption(options, "range");
76+
bool overwrite_sheet = GetBoolOption(options, "overwrite_sheet", true).first;
77+
bool overwrite_range = GetBoolOption(options, "overwrite_range", false).first;
78+
bool create_if_not_exists = GetBoolOption(options, "create_if_not_exists", false).first;
12979

130-
const auto create_if_not_exists_opt = options.find("create_if_not_exists");
131-
bool create_if_not_exists;
80+
auto header_result = GetBoolOption(options, "header", true);
81+
bool header = header_result.second ? header_result.first : (overwrite_range || overwrite_sheet);
13282

133-
if (create_if_not_exists_opt != options.end()) {
134-
if (create_if_not_exists_opt->second.size() != 1) {
135-
throw BinderException("create_if_not_exists option must be a single boolean value");
136-
}
137-
string error_msg;
138-
Value create_if_not_exists_val;
139-
if (!create_if_not_exists_opt->second.back().DefaultTryCastAs(LogicalType::BOOLEAN, create_if_not_exists_val,
140-
&error_msg)) {
141-
throw BinderException("Failed to cast option to bool: ", error_msg);
142-
}
143-
if (create_if_not_exists_val.IsNull()) {
144-
throw BinderException("Option must not be null");
145-
}
146-
create_if_not_exists = BooleanValue::Get(create_if_not_exists_val);
147-
if (create_if_not_exists && sheet.empty()) {
148-
throw BinderException("Must provide sheet name");
149-
}
150-
} else {
151-
create_if_not_exists = false;
83+
if (create_if_not_exists && sheet.empty()) {
84+
throw BinderException("Must provide sheet name");
15285
}
15386

15487
return make_uniq<GSheetWriteBindData>(file_path, sql_types, names, sheet, range, overwrite_sheet, overwrite_range,

0 commit comments

Comments
 (0)