Skip to content

Commit bb90cad

Browse files
ldanilekConvex, Inc.
authored and
Convex, Inc.
committed
user error when search on nonexistent table (#27246)
when we don't have a tablet index name, the index is missing and we have an index name to use in an error message. hence the unusual `Result` type added regression test. GitOrigin-RevId: 7127845a9895525fb9cd7d88bd87ab8a8725a1c1
1 parent 29a3560 commit bb90cad

File tree

6 files changed

+83
-29
lines changed

6 files changed

+83
-29
lines changed

crates/common/src/types/index.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,23 @@ pub type TabletIndexName = GenericIndexName<TabletId>;
128128
pub enum StableIndexName {
129129
Physical(TabletIndexName),
130130
Virtual(IndexName, TabletIndexName),
131-
Missing,
131+
Missing(IndexName),
132132
}
133133

134134
impl StableIndexName {
135135
pub fn tablet_index_name(&self) -> Option<&TabletIndexName> {
136136
match self {
137137
StableIndexName::Physical(tablet_index_name) => Some(tablet_index_name),
138138
StableIndexName::Virtual(_, tablet_index_name) => Some(tablet_index_name),
139-
StableIndexName::Missing => None,
139+
StableIndexName::Missing(_) => None,
140+
}
141+
}
142+
143+
pub fn tablet_index_name_or_missing(&self) -> Result<&TabletIndexName, &IndexName> {
144+
match self {
145+
StableIndexName::Physical(tablet_index_name) => Ok(tablet_index_name),
146+
StableIndexName::Virtual(_, tablet_index_name) => Ok(tablet_index_name),
147+
StableIndexName::Missing(index_name) => Err(index_name),
140148
}
141149
}
142150

@@ -160,7 +168,7 @@ impl StableIndexName {
160168
.tablet_number(*tablet_index_name.table())?,
161169
}))
162170
},
163-
StableIndexName::Missing => Ok(None),
171+
StableIndexName::Missing(_) => Ok(None),
164172
}
165173
}
166174
}

crates/database/src/bootstrap_model/index.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,7 @@ impl<'a, RT: Runtime> IndexModel<'a, RT> {
680680
)),
681681
TableFilter::ExcludePrivateSystemTables => {
682682
if index_name.table().is_system() {
683-
Ok(StableIndexName::Missing)
683+
Ok(StableIndexName::Missing(index_name.clone()))
684684
} else {
685685
Ok(StableIndexName::Physical(
686686
self.resolve_index_name(namespace, index_name)?,
@@ -689,7 +689,7 @@ impl<'a, RT: Runtime> IndexModel<'a, RT> {
689689
},
690690
}
691691
} else {
692-
Ok(StableIndexName::Missing)
692+
Ok(StableIndexName::Missing(index_name.clone()))
693693
}
694694
}
695695

crates/database/src/bootstrap_model/user_facing.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ fn start_index_range<RT: Runtime>(
340340
max_size: max_rows,
341341
}))
342342
},
343-
StableIndexName::Missing => Ok(Ok(DeveloperIndexRangeResponse {
343+
StableIndexName::Missing(_) => Ok(Ok(DeveloperIndexRangeResponse {
344344
page: vec![],
345345
cursor: CursorPosition::End,
346346
})),

crates/database/src/query/search_query.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use common::{
2020
},
2121
};
2222
use errors::ErrorMetadata;
23+
use indexing::index_registry::index_not_found_error;
2324
use search::{
2425
CandidateRevision,
2526
MAX_CANDIDATE_REVISIONS,
@@ -97,15 +98,17 @@ impl SearchQuery {
9798
.into_iter()
9899
.filter(|(_, index_key)| self.cursor_interval.contains(index_key))
99100
.collect();
100-
let namespace = match self.stable_index_name.tablet_index_name() {
101-
Some(index_name) => tx.table_mapping().tablet_namespace(*index_name.table())?,
102-
None => TableNamespace::Global,
101+
let (namespace, table_number) = match self.stable_index_name.tablet_index_name_or_missing()
102+
{
103+
Ok(index_name) => {
104+
let namespace = tx.table_mapping().tablet_namespace(*index_name.table())?;
105+
let tablet_number = tx.table_mapping().tablet_number(*index_name.table())?;
106+
(namespace, tablet_number)
107+
},
108+
Err(missing_index_name) => {
109+
anyhow::bail!(index_not_found_error(missing_index_name));
110+
},
103111
};
104-
let table_number = tx
105-
.table_mapping()
106-
.namespace(namespace)
107-
.id(&self.query.table)?
108-
.table_number;
109112
Ok(SearchResultIterator::new(
110113
revisions_in_range,
111114
namespace,

crates/isolate/src/tests/user_error.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,16 @@ async fn test_nonexistent_table(rt: TestRuntime) -> anyhow::Result<()> {
161161
.await
162162
}
163163

164+
#[convex_macro::test_runtime]
165+
async fn test_index_on_nonexistent_table(rt: TestRuntime) -> anyhow::Result<()> {
166+
UdfTest::run_test_with_isolate2(rt, async move |t: UdfTestType| {
167+
t.mutation("userError:indexOnNonexistentTable", assert_obj!())
168+
.await?;
169+
Ok(())
170+
})
171+
.await
172+
}
173+
164174
#[convex_macro::test_runtime]
165175
async fn test_nonexistent_id(rt: TestRuntime) -> anyhow::Result<()> {
166176
UdfTest::run_test_with_isolate2(rt, async move |t: UdfTestType| {

npm-packages/udf-tests/convex/userError.ts

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -124,21 +124,6 @@ export const nonexistentTable = mutation(
124124
"expected query filtering to fake ID to return 0 results",
125125
);
126126
}
127-
// Querying a non-existent table is fine.
128-
results = await db.query("missing" as any).collect();
129-
if (results.length !== 0) {
130-
throw new Error('expected db.query("missing") to return 0 results');
131-
}
132-
133-
// Querying a non-existent table with filters is fine too
134-
results = await db
135-
.query("missing" as any)
136-
.filter((q) => q.eq(q.field("foo"), "foo"))
137-
.collect();
138-
if (results.length !== 0) {
139-
throw new Error('expected db.query("missing") to return 0 results');
140-
}
141-
142127
results = await db
143128
.query("boatVotes")
144129
.withIndex("by_boat", (q) => q.gt("boat", fakeId))
@@ -151,6 +136,54 @@ export const nonexistentTable = mutation(
151136
},
152137
);
153138

139+
export const indexOnNonexistentTable = mutation(async ({ db }) => {
140+
// Querying a non-existent table by_creation_time is fine.
141+
let results = await db.query("missing" as any).collect();
142+
if (results.length !== 0) {
143+
throw new Error('expected db.query("missing") to return 0 results');
144+
}
145+
146+
// Querying a non-existent table with filters is fine too
147+
results = await db
148+
.query("missing" as any)
149+
.filter((q) => q.eq(q.field("foo"), "foo"))
150+
.collect();
151+
if (results.length !== 0) {
152+
throw new Error('expected db.query("missing") to return 0 results');
153+
}
154+
155+
// Querying a non-existent table with index throws error about the index
156+
// missing (not the table missing).
157+
const assertIndexNotFound = async (
158+
thunk: () => Promise<void>,
159+
indexName: string,
160+
) => {
161+
try {
162+
await thunk();
163+
} catch (error: any) {
164+
if (!error.toString().includes(`Index ${indexName} not found`)) {
165+
throw error;
166+
}
167+
return;
168+
}
169+
throw new Error("expected TableNotFound error");
170+
};
171+
await assertIndexNotFound(async () => {
172+
await db
173+
.query("missing" as any)
174+
.withIndex("by_foo")
175+
.collect();
176+
}, "missing.by_foo");
177+
178+
// Same error with search index
179+
await assertIndexNotFound(async () => {
180+
await db
181+
.query("missing" as any)
182+
.withSearchIndex("search_foo", (q) => q.search("foo", "foo"))
183+
.collect();
184+
}, "missing.search_foo");
185+
});
186+
154187
export const nonexistentId = mutation({
155188
handler: async (
156189
{ db },

0 commit comments

Comments
 (0)