[SERVER-62322] Consistent validity treatment of empty objects (i.e., `{}`) Created: 30/Dec/21  Updated: 29/Oct/23  Resolved: 25/Jan/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 5.3.0

Type: Improvement Priority: Major - P3
Reporter: Mohammad Dashti (Inactive) Assignee: Nicholas Zolnierz
Resolution: Fixed Votes: 0
Labels: auto-reverted
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Depends
Problem/Incident
Related
is related to SERVER-63079 Avoid using projection parser in $set... Closed
Backwards Compatibility: Fully Compatible
Backport Requested:
v5.2
Participants:
Linked BF Score: 173

 Description   

Currently, there are two places in the code (i.e., here and here) that empty objects are strictly rejected in a user/intermediate document via assertions with this message: "an empty object is not a valid value". As empty objects are valid values, the reason for these assertions in the abovementioned code segments is unclear.

Now, we have two options in front of us:

1- Keep these assertions: In this case, we need to make sure that valid user documents are cleaned before getting to these assertions (and make sure they don't have empty objects at any level). In addition, we need to make sure that no empty object is produced as an intermediate document (i.e., a result of previous stages).

Here's an example for the latter:
Currently, it's possible to produce empty objects using $project. Here's a small program to show this:

(function () {
'use strict';
 
const coll = db.test_bf;
coll.drop();
 
const aggregationList = [
  [
    {
      $project:{
        "obj.date":null,
        "obj.obj.str":1,
        "obj.num":1
      }
    },
    {
      $setWindowFields:{
        output:{
          obj:{
            $max:{
              $mergeObjects:[
                "$obj"
              ]
            }
          }
        }
      }
    }
  ]
];
 
const documentList = [
  {
    "obj":{
      "num":NumberLong("34921"),
      "obj":{
        "num":NumberLong("34922"),
      },
    },
  },
  {
    "obj":{
      "num":NumberLong("34920"),
    },
  },
];
 
assert.commandWorked(coll.insertMany(documentList));
 
const res = coll.aggregate(aggregationList[0]).toArray();
print("Result:");
printjson(res);
 
}());

Running this program on 5.2.0-rc2@4fab480 produces this output:

Result:
[
	{
		"_id" : ObjectId("61ce0c951237cda2512da341"),
		"obj" : {
			"num" : NumberLong(34921),
			"date" : null,
			"obj" : {
				
			}
		}
	},
	{
		"_id" : ObjectId("61ce0c951237cda2512da342"),
		"obj" : {
			"num" : NumberLong(34920),
			"date" : null
		}
	}
]

As you observe, an empty object is produced in the first result. Now, if we run this program instead (which adds a $setWindowFields at the end of pipeline):

(function () {
'use strict';
 
const coll = db.test_bf;
coll.drop();
 
const aggregationList = [
  [
    {
      $project:{
        "obj.date":null,
        "obj.obj.str":1,
        "obj.num":1
      }
    },
    {
      $setWindowFields:{
        output:{
          obj:{
            $max:{
              $mergeObjects:[
                "$obj"
              ]
            }
          }
        }
      }
    }
  ]
];
 
const documentList = [
  {
    "obj":{
      "num":NumberLong("34921"),
      "obj":{
        "num":NumberLong("34922"),
      },
    },
  },
  {
    "obj":{
      "num":NumberLong("34920"),
    },
  },
];
 
assert.commandWorked(coll.insertMany(documentList));
 
const res = coll.aggregate(aggregationList[0]).toArray();
print("Result:");
printjson(res);
 
}());

Running the program produces the following error:

uncaught exception: Error: command failed: {
	"ok" : 0,
	"errmsg" : "PlanExecutor error during aggregation :: caused by :: an empty object is not a valid value. Found empty object at path obj.obj >>> {}",
	"code" : 40180,
	"codeName" : "Location40180"
} with original command request: {
	"aggregate" : "test_bf",
	"pipeline" : [
		{
			"$project" : {
				"obj.date" : null,
				"obj.obj.str" : 1,
				"obj.num" : 1
			}
		},
		{
			"$setWindowFields" : {
				"output" : {
					"obj" : {
						"$max" : {
							"$mergeObjects" : [
								"$obj"
							]
						}
					}
				}
			}
		}
	],
	"cursor" : {
		
	},
	"lsid" : {
		"id" : UUID("bf0b9111-f13b-44ed-8d3e-f3784c5a3df8")
	}
}

2- Remove these assertions and fix the rest of the code that depends on this assumption (i.e., there's no empty object in the input).

IMO, with enough caution, taking the second option makes more sense.

anton.korshunov as it seems that you have added these assertions, it'd be great if you could advise on the best way to deal with this issue.



 Comments   
Comment by Githook User [ 03/Feb/22 ]

Author:

{'name': 'HanaPearlman', 'email': 'hana.pearlman@mongodb.com', 'username': 'HanaPearlman'}

Message: TIG-3800: Multiversion fuzzer should ignore differences in output after SERVER-62322 (#641)
Branch: master
https://github.com/10gen/jstestfuzz/commit/e71ca3d75e8d7012d6f44efd825e6fbdefaaa558

Comment by Githook User [ 21/Jan/22 ]

Author:

{'name': 'Nicholas Zolnierz', 'email': 'nicholas.zolnierz@mongodb.com', 'username': 'nzolnierzmdb'}

Message: SERVER-62322 Allow an empty object to be projected in $setWindowFields
Branch: master
https://github.com/mongodb/mongo/commit/b02796c77befd906e90ed7c1cd764c888d90524c

Comment by xgen-buildbaron-user [ 21/Jan/22 ]

Ticket re-opened due to revert. aggregation began a consistent failure of jstests/aggregation/sources/fill/fill_and_densify.js,jstests/aggregation/sources/setWindowFields/locf.js

Comment by Githook User [ 21/Jan/22 ]

Author:

{'name': 'auto-revert-processor', 'email': 'dev-prod-dag@mongodb.com'}

Message: Revert "SERVER-62322 Allow an empty object to be projected in $setWindowFields"

This reverts commit 2dac3294841b0374a94a5823f3726d5d02e27261.
Branch: master
https://github.com/mongodb/mongo/commit/2ea6389bf7069644616b3ab7b9f67002c8554870

Comment by Nicholas Zolnierz [ 20/Jan/22 ]

Note that the fix was isolated to $setWindowFields so the title/description may be misleading.

Comment by Githook User [ 20/Jan/22 ]

Author:

{'name': 'Nicholas Zolnierz', 'email': 'nicholas.zolnierz@mongodb.com', 'username': 'nzolnierzmdb'}

Message: SERVER-62322 Allow an empty object to be projected in $setWindowFields
Branch: master
https://github.com/mongodb/mongo/commit/2dac3294841b0374a94a5823f3726d5d02e27261

Comment by Githook User [ 19/Jan/22 ]

Author:

{'name': 'Nicholas Zolnierz', 'email': 'nicholas.zolnierz@mongodb.com', 'username': 'nzolnierzmdb'}

Message: SERVER-62322 Allow an empty object to be projected in $setWindowFields
Branch: server-62322-setWindowFields-empty-obj
https://github.com/mongodb/mongo/commit/76d67ea903cdad9cb574f197ba04e63b0747a3ca

Comment by Mohammad Dashti (Inactive) [ 13/Jan/22 ]

nicholas.zolnierz Thanks for looking into this ticket and giving some pointers to the related issues in the past. wrapping the projected field in a $literal sounds like a working solution. Although, my initial instinct was to fix the code re-use of projection machinery in $setWindowFields by adding some flag to the projection code to disable its syntax checks in this case (or any similar code- re-use that involves user data).

Comment by Nicholas Zolnierz [ 13/Jan/22 ]

This has come up a few times in the past (SERVER-28791, SERVER-46422), and it looks like we tightened up these checks in 3.4 presumably because of the reasons described above (ambiguity and the fact that nested objects are the way that nested fields are projected). I don't think we're going to change the behavior of the normal projection/addFields code. There is still a bug in the usage of the projection machinery in setWindowFields, but I'm not quite sure of the best way to fix it. One option is to wrap the projected field in a $literal. mohammad.dashti does that sound good to you?

Also note that DOCS-13489 aims to make this behavior a bit more obvious in the documentation.

Comment by Joe Kanaan [ 13/Jan/22 ]

nicholas.zolnierz, can you take a look at this please?

Comment by Mohammad Dashti (Inactive) [ 06/Jan/22 ]

david.percy I think the initial purpose was to assert the syntax and "disallow `{$project: {}}` because it's unclear whether it's an inclusion or exclusion projection.", as you've suggested. However, that code has been re-used in other places (e.g., in the implementation of setWindowFields)), where input user documents are treated as the syntax, e.g., similar to the examples in the ticket description.

Comment by David Percy [ 06/Jan/22 ]

I thought those assertions are about syntax, not values. I think we disallow `{$project: {}}` because it's unclear whether it's an inclusion or exclusion projection.

Maybe the bug is that we're generating that invalid syntax internally, during some optimization (or in parsing, desugaring, or serialization).

Comment by Joe Kanaan [ 06/Jan/22 ]

anton.korshunov, can you please take a look at this? Thanks

Generated at Thu Feb 08 05:54:48 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.