From 4ec408b35d7b4748e814723621fbbea0eaf93320 Mon Sep 17 00:00:00 2001 From: h3n4l Date: Thu, 7 May 2026 16:40:18 +0800 Subject: [PATCH 1/2] feat: support db.coll.count() and cursor count/itcount/size Adds the deprecated-but-heavily-used count() forms by routing through the modern driver methods: - db.coll.count() (zero args) -> EstimatedDocumentCount, preserving the fast metadata path mongosh users expect on large collections. - db.coll.count(filter, opts?) -> CountDocuments, the only correct option once a filter is involved (EstimatedDocumentCount cannot take a filter). - cursor.count() / itcount() / size() on a find cursor -> CountDocuments with the accumulated filter/skip/limit/hint. Modern mongosh always honors skip/limit on cursor.count, so we do too. Aggregate-cursor itcount() requires pipeline rewriting ($count stage) and is left unsupported until telemetry shows demand. Motivated by gomongo-fallback telemetry from production: count() forms account for the largest single bucket of mongosh fallbacks. Removing them is a prerequisite for retiring the mongosh fallback path. Co-Authored-By: Claude Opus 4.7 (1M context) --- collection_test.go | 157 ++++++++++++++++++++++++++++++- internal/translator/translate.go | 23 +++++ 2 files changed, 176 insertions(+), 4 deletions(-) diff --git a/collection_test.go b/collection_test.go index 7d911a1..6b229a5 100644 --- a/collection_test.go +++ b/collection_test.go @@ -1943,6 +1943,111 @@ func TestEstimatedDocumentCountMaxTimeMS(t *testing.T) { }) } +// TestCollectionCount covers db.coll.count() — the deprecated collection method +// still heavily used in mongosh scripts. Zero-arg routes to estimatedDocumentCount +// (preserving the fast metadata path); any-arg routes to countDocuments. +func TestCollectionCount(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_coll_count_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + coll := db.Client.Database(dbName).Collection("users") + _, err := coll.InsertMany(ctx, []any{ + bson.M{"name": "alice", "age": 30, "status": "active"}, + bson.M{"name": "bob", "age": 25, "status": "inactive"}, + bson.M{"name": "charlie", "age": 35, "status": "active"}, + bson.M{"name": "diana", "age": 28, "status": "active"}, + }) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // count() — zero args, fast path + result, err := gc.Execute(ctx, dbName, "db.users.count()") + require.NoError(t, err) + require.Equal(t, int64(4), result.Value[0].(int64)) + + // count({}) — explicit empty filter, accurate path + result, err = gc.Execute(ctx, dbName, "db.users.count({})") + require.NoError(t, err) + require.Equal(t, int64(4), result.Value[0].(int64)) + + // count(filter) + result, err = gc.Execute(ctx, dbName, `db.users.count({ status: "active" })`) + require.NoError(t, err) + require.Equal(t, int64(3), result.Value[0].(int64)) + + // count(filter) with comparison operator + result, err = gc.Execute(ctx, dbName, `db.users.count({ age: { $gte: 30 } })`) + require.NoError(t, err) + require.Equal(t, int64(2), result.Value[0].(int64)) + }) +} + +func TestCollectionCountWithOptions(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_coll_count_opts_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + coll := db.Client.Database(dbName).Collection("users") + _, err := coll.InsertMany(ctx, []any{ + bson.M{"name": "alice", "age": 30}, + bson.M{"name": "bob", "age": 25}, + bson.M{"name": "charlie", "age": 35}, + bson.M{"name": "diana", "age": 28}, + bson.M{"name": "eve", "age": 40}, + }) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // limit option + result, err := gc.Execute(ctx, dbName, `db.users.count({}, { limit: 3 })`) + require.NoError(t, err) + require.Equal(t, int64(3), result.Value[0].(int64)) + + // skip option + result, err = gc.Execute(ctx, dbName, `db.users.count({}, { skip: 2 })`) + require.NoError(t, err) + require.Equal(t, int64(3), result.Value[0].(int64)) + + // skip + limit + result, err = gc.Execute(ctx, dbName, `db.users.count({}, { skip: 1, limit: 2 })`) + require.NoError(t, err) + require.Equal(t, int64(2), result.Value[0].(int64)) + + // maxTimeMS option + result, err = gc.Execute(ctx, dbName, `db.users.count({}, { maxTimeMS: 5000 })`) + require.NoError(t, err) + require.Equal(t, int64(5), result.Value[0].(int64)) + }) +} + +func TestCollectionCountEmptyCollection(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_coll_count_empty_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + gc := gomongo.NewClient(db.Client) + + // Zero-arg count on missing collection + result, err := gc.Execute(ctx, dbName, "db.users.count()") + require.NoError(t, err) + require.Equal(t, int64(0), result.Value[0].(int64)) + + // count({}) on missing collection + result, err = gc.Execute(ctx, dbName, "db.users.count({})") + require.NoError(t, err) + require.Equal(t, int64(0), result.Value[0].(int64)) + }) +} + func TestDistinct(t *testing.T) { testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { dbName := fmt.Sprintf("testdb_distinct_%s", db.Name) @@ -2116,22 +2221,66 @@ func TestDistinctMaxTimeMS(t *testing.T) { }) } -func TestCursorCountUnsupported(t *testing.T) { +func TestCursorCount(t *testing.T) { testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { dbName := fmt.Sprintf("testdb_cursor_count_%s", db.Name) defer testutil.CleanupDatabase(t, db.Client, dbName) ctx := context.Background() + coll := db.Client.Database(dbName).Collection("users") + _, err := coll.InsertMany(ctx, []any{ + bson.M{"name": "Alice", "status": "active"}, + bson.M{"name": "Bob", "status": "inactive"}, + bson.M{"name": "Charlie", "status": "active"}, + bson.M{"name": "Diana", "status": "active"}, + }) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // find().count() — empty filter + result, err := gc.Execute(ctx, dbName, "db.users.find().count()") + require.NoError(t, err) + require.Equal(t, int64(4), result.Value[0].(int64)) + + // find(filter).count() — accumulates filter from find() + result, err = gc.Execute(ctx, dbName, `db.users.find({ status: "active" }).count()`) + require.NoError(t, err) + require.Equal(t, int64(3), result.Value[0].(int64)) + + // find().skip().limit().count() — modern mongosh always honors skip/limit on cursor.count + result, err = gc.Execute(ctx, dbName, "db.users.find().skip(1).limit(2).count()") + require.NoError(t, err) + require.Equal(t, int64(2), result.Value[0].(int64)) + + // itcount and size are aliases for count on a find cursor + result, err = gc.Execute(ctx, dbName, `db.users.find({ status: "active" }).itcount()`) + require.NoError(t, err) + require.Equal(t, int64(3), result.Value[0].(int64)) + + result, err = gc.Execute(ctx, dbName, `db.users.find({ status: "active" }).size()`) + require.NoError(t, err) + require.Equal(t, int64(3), result.Value[0].(int64)) + }) +} + +func TestAggregateItcountUnsupported(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_agg_itcount_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + gc := gomongo.NewClient(db.Client) - // cursor.count() is not in the planned registry, should return UnsupportedOperationError - _, err := gc.Execute(ctx, dbName, "db.users.find().count()") + // aggregate cursors expose itcount() in mongosh but require pipeline rewriting + // ($count stage). Out of scope until telemetry shows demand. + _, err := gc.Execute(ctx, dbName, "db.users.aggregate([{ $match: {} }]).itcount()") require.Error(t, err) var unsupportedErr *gomongo.UnsupportedOperationError require.ErrorAs(t, err, &unsupportedErr) - require.Equal(t, "count()", unsupportedErr.Operation) }) } diff --git a/internal/translator/translate.go b/internal/translator/translate.go index 6d8b5e0..e970c6d 100644 --- a/internal/translator/translate.go +++ b/internal/translator/translate.go @@ -89,6 +89,19 @@ func translateCollectionStatement(op *Operation, stmt *ast.CollectionStatement) if err := extractEstimatedDocumentCountArgs(op, stmt.Args); err != nil { return nil, err } + case "count": + // db.coll.count() is deprecated in mongosh in favor of countDocuments / + // estimatedDocumentCount. We honor both halves of that recommendation: + // zero-arg count() routes to estimatedDocumentCount (preserves the fast + // metadata path), and any-arg count() routes to countDocuments. + if len(stmt.Args) == 0 { + op.OpType = types.OpEstimatedDocumentCount + } else { + op.OpType = types.OpCountDocuments + if err := extractCountDocumentsArgs(op, stmt.Args); err != nil { + return nil, err + } + } case "distinct": op.OpType = types.OpDistinct if err := extractDistinctArgs(op, stmt.Args); err != nil { @@ -239,6 +252,16 @@ func translateCursorMethod(op *Operation, cm ast.CursorMethod) error { return extractMin(op, cm.Args) case "pretty": return nil // no-op + case "count", "itcount", "size": + // Cursor terminals on a find cursor: count documents matching the + // accumulated filter, honoring skip/limit/hint. Aggregate cursors also + // expose itcount() but require pipeline rewriting ($count stage); not + // yet supported. + if op.OpType != types.OpFind { + return &UnsupportedOperationError{Operation: cm.Method + "()"} + } + op.OpType = types.OpCountDocuments + return nil default: return &UnsupportedOperationError{Operation: cm.Method + "()"} } From 5a84d0e4e1b8550ab1d20bd7882dcab44296e211 Mon Sep 17 00:00:00 2001 From: h3n4l Date: Thu, 7 May 2026 17:30:55 +0800 Subject: [PATCH 2/2] review: reject cursor count/itcount/size args; tighten unsupported assert Address Copilot review feedback on PR #28. - translate.go: cursor count/itcount/size now reject any positional arguments rather than silently dropping them. mongosh's legacy cursor.count(applySkipLimit) boolean is a no-op in modern drivers (skip/limit always apply), so accepting it would make incorrect caller assumptions hard to debug. Updated comment to spell out *why* the OpType is retargeted instead of running find then count on the cursor (mongosh issues a separate count command, never iterates). - collection_test.go: TestAggregateItcountUnsupported now asserts the Operation field on the error so a different unsupported method can't slip past as a false positive. - Adds TestCursorCountRejectsArgs covering count(true)/count(false)/ itcount(true)/size(1). Co-Authored-By: Claude Opus 4.7 (1M context) --- collection_test.go | 27 +++++++++++++++++++++++++++ internal/translator/translate.go | 16 ++++++++++++---- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/collection_test.go b/collection_test.go index 6b229a5..c2235ec 100644 --- a/collection_test.go +++ b/collection_test.go @@ -2281,6 +2281,33 @@ func TestAggregateItcountUnsupported(t *testing.T) { var unsupportedErr *gomongo.UnsupportedOperationError require.ErrorAs(t, err, &unsupportedErr) + require.Equal(t, "itcount()", unsupportedErr.Operation) + }) +} + +// TestCursorCountRejectsArgs guards against mongosh's legacy +// cursor.count(applySkipLimit) form silently dropping the boolean: the arg is +// a no-op in modern drivers, and silently dropping it would make incorrect +// caller assumptions hard to debug. +func TestCursorCountRejectsArgs(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_cursor_count_args_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + gc := gomongo.NewClient(db.Client) + + for _, stmt := range []string{ + "db.users.find().count(true)", + "db.users.find().count(false)", + "db.users.find().itcount(true)", + "db.users.find().size(1)", + } { + _, err := gc.Execute(ctx, dbName, stmt) + require.Error(t, err, "expected %q to be rejected", stmt) + require.Contains(t, err.Error(), "takes no arguments") + } }) } diff --git a/internal/translator/translate.go b/internal/translator/translate.go index e970c6d..06f57e6 100644 --- a/internal/translator/translate.go +++ b/internal/translator/translate.go @@ -253,10 +253,18 @@ func translateCursorMethod(op *Operation, cm ast.CursorMethod) error { case "pretty": return nil // no-op case "count", "itcount", "size": - // Cursor terminals on a find cursor: count documents matching the - // accumulated filter, honoring skip/limit/hint. Aggregate cursors also - // expose itcount() but require pipeline rewriting ($count stage); not - // yet supported. + // mongosh's cursor.count() never iterates the cursor; it issues a + // separate count command server-side. We mirror that by retargeting + // the operation to CountDocuments with the accumulated + // filter+skip+limit+hint. Aggregate cursors also expose itcount() but + // require pipeline rewriting ($count stage); not yet supported. + if len(cm.Args) > 0 { + // mongosh historically accepted cursor.count(applySkipLimit), but + // the boolean is a no-op in modern drivers (skip/limit always + // apply). Reject rather than silently drop, since "no-op" is + // hard to debug. + return fmt.Errorf("%s() takes no arguments", cm.Method) + } if op.OpType != types.OpFind { return &UnsupportedOperationError{Operation: cm.Method + "()"} }