Skip to content

Commit e19e189

Browse files
rustyconoverclaude
andcommitted
feat(attach): discover catalog from bare connection-string LOCATION
Enable the bare connection-string form of VGI ATTACH so the haybarn CLI DSN `vgi:uvx vgi-easter` works. When no LOCATION is given and the ATTACH path carries no `?query` (which marks the base as a catalog name, airport-style), treat the whole path as the worker location and discover the remote catalog name via the existing InvokeCatalogs RPC. A single-catalog worker resolves unambiguously; a worker exposing >1 catalog throws a BinderException naming them and pointing at the disambiguating forms. The `?query`-without-location case still errors with the LOCATION message, preserving the existing contract. The catalog list is now fetched once and reused for both discovery and attach-option validation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0504226 commit e19e189

2 files changed

Lines changed: 106 additions & 38 deletions

File tree

src/vgi_extension.cpp

Lines changed: 83 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,9 +1156,11 @@ static unique_ptr<Catalog> VgiCatalogAttach(optional_ptr<StorageExtensionInfo> s
11561156
// The base (before '?') stays the catalog name; the query string supplies
11571157
// options, applied first so an explicit (TYPE vgi, ...) clause still overrides.
11581158
bool secret_in_path = false;
1159+
bool had_query = false;
11591160
{
11601161
auto qpos = catalog_name.find('?');
11611162
if (qpos != string::npos) {
1163+
had_query = true;
11621164
string query = catalog_name.substr(qpos + 1);
11631165
catalog_name = catalog_name.substr(0, qpos);
11641166
size_t start = 0;
@@ -1207,6 +1209,18 @@ static unique_ptr<Catalog> VgiCatalogAttach(optional_ptr<StorageExtensionInfo> s
12071209
throw BinderException("Cannot specify both bearer_token and oauth_refresh_token");
12081210
}
12091211

1212+
// Bare connection-string form: when no LOCATION was given and the path
1213+
// carried no '?query' (which would mark the base as a catalog name,
1214+
// airport-style), the whole ATTACH path IS the worker location — e.g. the
1215+
// haybarn CLI form `vgi:uvx vgi-easter`, or `ATTACH 'uvx vgi-easter'
1216+
// (TYPE vgi)`. The remote catalog name is discovered from the worker below.
1217+
bool discover_catalog = false;
1218+
if (worker_path.empty() && !had_query && !catalog_name.empty()) {
1219+
worker_path = catalog_name;
1220+
catalog_name.clear();
1221+
discover_catalog = true;
1222+
}
1223+
12101224
if (worker_path.empty()) {
12111225
throw BinderException("VGI ATTACH requires LOCATION option specifying the worker path");
12121226
}
@@ -1288,57 +1302,88 @@ static unique_ptr<Catalog> VgiCatalogAttach(optional_ptr<StorageExtensionInfo> s
12881302
launcher_state_dir_for_attach = launcher_state_dir;
12891303
}
12901304

1291-
// If the user passed attach-time options, validate them against the
1292-
// catalog's declared AttachOptionSpec list. Skip the extra RPC when
1293-
// attach_options is empty — catalogs without options pay no overhead.
1294-
if (!attach_options.empty()) {
1305+
// Fetch the worker's catalog list once when we need it — either to discover
1306+
// the catalog name for the bare connection-string form, or to validate
1307+
// attach-time options against the catalog's declared AttachOptionSpec list.
1308+
// Catalogs without options on the explicit form pay no overhead (no RPC).
1309+
if (discover_catalog || !attach_options.empty()) {
12951310
auto catalogs = vgi::InvokeCatalogs(worker_path, context, worker_debug, use_pool, auth,
12961311
launcher_idle_for_attach, launcher_state_dir_for_attach);
1297-
const vgi::VgiCatalogInfo *matching_info = nullptr;
1298-
for (const auto &info_entry : catalogs) {
1299-
if (info_entry.name == catalog_name) {
1300-
matching_info = &info_entry;
1301-
break;
1302-
}
1303-
}
1304-
if (!matching_info) {
1305-
throw BinderException("Worker at '%s' exposes no catalog named '%s'", worker_path, catalog_name);
1306-
}
13071312

1308-
auto build_accepted_list = [&]() {
1309-
std::string joined;
1310-
for (const auto &spec : matching_info->attach_option_specs) {
1311-
if (!joined.empty()) {
1312-
joined += ", ";
1313+
// Bare form: resolve the (omitted) catalog name from the worker. A
1314+
// single-catalog worker resolves unambiguously; >1 forces the user to
1315+
// name one via the '<catalog>?location=<worker>' form.
1316+
if (discover_catalog) {
1317+
if (catalogs.empty()) {
1318+
throw BinderException("VGI worker at '%s' exposes no catalogs", worker_path);
1319+
}
1320+
if (catalogs.size() > 1) {
1321+
std::string names;
1322+
for (const auto &c : catalogs) {
1323+
if (!names.empty()) {
1324+
names += ", ";
1325+
}
1326+
names += c.name;
13131327
}
1314-
joined += spec.name;
1328+
throw BinderException(
1329+
"VGI worker at '%s' exposes %llu catalogs (%s); name one with "
1330+
"ATTACH '<catalog>?location=%s' (TYPE vgi) or "
1331+
"ATTACH '<catalog>' (TYPE vgi, LOCATION '%s')",
1332+
worker_path, static_cast<unsigned long long>(catalogs.size()), names, worker_path,
1333+
worker_path);
13151334
}
1316-
return joined.empty() ? std::string("(none)") : joined;
1317-
};
1335+
catalog_name = catalogs[0].name;
1336+
}
13181337

1319-
std::map<std::string, Value> validated_options;
1320-
for (const auto &opt : attach_options) {
1321-
const vgi::VgiAttachOptionSpec *matching_spec = nullptr;
1322-
for (const auto &spec : matching_info->attach_option_specs) {
1323-
if (StringUtil::Lower(spec.name) == opt.first) {
1324-
matching_spec = &spec;
1338+
// Validate attach-time options against the resolved catalog's declared
1339+
// AttachOptionSpec list (skipped on the bare form when no options given).
1340+
if (!attach_options.empty()) {
1341+
const vgi::VgiCatalogInfo *matching_info = nullptr;
1342+
for (const auto &info_entry : catalogs) {
1343+
if (info_entry.name == catalog_name) {
1344+
matching_info = &info_entry;
13251345
break;
13261346
}
13271347
}
1328-
if (!matching_spec) {
1329-
throw BinderException("Unknown ATTACH option '%s' for catalog '%s'. Accepted options: %s",
1330-
opt.first, catalog_name, build_accepted_list());
1348+
if (!matching_info) {
1349+
throw BinderException("Worker at '%s' exposes no catalog named '%s'", worker_path, catalog_name);
13311350
}
13321351

1333-
Value casted;
1334-
std::string cast_error;
1335-
if (!opt.second.DefaultTryCastAs(matching_spec->type, casted, &cast_error)) {
1336-
throw BinderException("Cannot cast ATTACH option '%s' to declared type %s: %s", matching_spec->name,
1337-
matching_spec->type.ToString(), cast_error);
1352+
auto build_accepted_list = [&]() {
1353+
std::string joined;
1354+
for (const auto &spec : matching_info->attach_option_specs) {
1355+
if (!joined.empty()) {
1356+
joined += ", ";
1357+
}
1358+
joined += spec.name;
1359+
}
1360+
return joined.empty() ? std::string("(none)") : joined;
1361+
};
1362+
1363+
std::map<std::string, Value> validated_options;
1364+
for (const auto &opt : attach_options) {
1365+
const vgi::VgiAttachOptionSpec *matching_spec = nullptr;
1366+
for (const auto &spec : matching_info->attach_option_specs) {
1367+
if (StringUtil::Lower(spec.name) == opt.first) {
1368+
matching_spec = &spec;
1369+
break;
1370+
}
1371+
}
1372+
if (!matching_spec) {
1373+
throw BinderException("Unknown ATTACH option '%s' for catalog '%s'. Accepted options: %s",
1374+
opt.first, catalog_name, build_accepted_list());
1375+
}
1376+
1377+
Value casted;
1378+
std::string cast_error;
1379+
if (!opt.second.DefaultTryCastAs(matching_spec->type, casted, &cast_error)) {
1380+
throw BinderException("Cannot cast ATTACH option '%s' to declared type %s: %s",
1381+
matching_spec->name, matching_spec->type.ToString(), cast_error);
1382+
}
1383+
validated_options.emplace(matching_spec->name, std::move(casted));
13381384
}
1339-
validated_options.emplace(matching_spec->name, std::move(casted));
1385+
attach_options = std::move(validated_options);
13401386
}
1341-
attach_options = std::move(validated_options);
13421387
}
13431388

13441389
// Call catalog_attach via RPC. The worker validates data_version_spec and

test/sql/integration/connection_string.test

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,30 @@ SELECT ex3.main.add_values(20, 22);
4444
42
4545

4646
# A '?' query that omits a location still fails with the clear LOCATION error.
47+
# (The '?' marks the base as a catalog name, airport-style — a location must be
48+
# supplied explicitly; it is NOT treated as a bare worker location.)
4749
statement error
4850
ATTACH 'example?pool=false' AS ex4 (TYPE vgi);
4951
----
5052
VGI ATTACH requires LOCATION option
53+
54+
# Bare connection-string form: with no '?query' and no LOCATION, the whole
55+
# ATTACH path IS the worker location and the remote catalog name is discovered
56+
# from the worker. This is the form behind the haybarn CLI's `vgi:uvx vgi-easter`.
57+
# The fixture worker exposes several catalogs, so discovery is ambiguous and the
58+
# error must name them and point at the disambiguating forms.
59+
statement error
60+
ATTACH '${VGI_TEST_WORKER}' AS exauto (TYPE vgi);
61+
----
62+
name one with ATTACH '<catalog>?location=
63+
64+
# Naming one of the discovered catalogs with the existing '?location=' form
65+
# resolves the ambiguity and attaches successfully (single-catalog discovery is
66+
# exercised end-to-end against a real single-catalog worker in the easter demo).
67+
statement ok
68+
ATTACH 'example?location=${VGI_TEST_WORKER}' AS exauto (TYPE vgi);
69+
70+
query I
71+
SELECT exauto.main.add_values(20, 22);
72+
----
73+
42

0 commit comments

Comments
 (0)