SERVER-5319 $strlen for aggregation framework #213

afchin · 2012-04-24T00:57:15Z

initial implementation and tests

cwestin · 2012-05-02T21:25:01Z

jstests/aggregation/testall.js

+assert(arrayEq(slen.result, slenresult), 'strlen1 failed');
+
+// $strlen on nonexistent field should return a "not a string"
+// error


I think $strlen on a nonexistent field should return zero. This fits the operation of other operators, which are as forgiving as possible unless the result is definitively wrong. Here, I think it makes sense to treat this as an empty string. However, I think it should be an error to call $strlen on a real field that does not contain a string value.

cwestin · 2012-05-02T21:41:21Z

Generally looks good, but as noted, I'd return zero for missing or null valued fields. Also, we shouldn't turn off those assertions, since they appear to work for everyone else. Can you describe what was causing them to go off for you? Did you look at the value of millis in the debugger and see what it is?

afchin · 2012-05-03T04:45:15Z

I uncommented the time stuff and it works now. I wish I knew why it works
now.

I added in the checks you recommended, but for some reason the tests just
keep skipping over to the assert line even for nonexistent fields.

intrusive_ptr<const Value> ExpressionStrlen::evaluate(
    const intrusive_ptr<Document> &pDocument) const {
    checkArgCount(1);
    intrusive_ptr<const Value>

pString(vpOperand[0]->evaluate(pDocument));

    // if the value is NULL or missing, return 0
    if ((pString.get() == NULL) || (pString->getType() == jstNULL)) {
        return Value::getZero();
    } else {
        uassert(16145, str::stream() << getOpName() <<
            ": input must be a string", pString->getType() == String);
        string str = pString->coerceToString();

        return Value::createInt(str.length());
    }

Thanks! (And I guess I'll see you on Friday!)

On Wed, May 2, 2012 at 2:41 PM, Chris Westin <
reply@reply.github.com

wrote:

Generally looks good, but as noted, I'd return zero for missing or null
valued fields. Also, we shouldn't turn off those assertions, since they
appear to work for everyone else. Can you describe what was causing them
to go off for you? Did you look at the value of millis in the debugger and
see what it is?

Reply to this email directly or view it on GitHub:
#213 (comment)

Annie™

cwestin · 2012-05-07T22:25:52Z

What do you mean "it skips over it?" Is it not executing the condition? That would be a sign of not having recompiled this file in the executable you're running (presuming you're checking this in the debugger). Or, are you saying the condition isn't true for non-existent fields? In that case, what is the type of pString for a non-existent field?

Either way, something sounds fishy here. Please dig into the details and let me know more.

afchin · 2012-05-08T03:48:05Z

the condition probably just isn't true, because i've recompiled everything
multiple times just to make sure.

i can't figure out how check the value because i can't get gdb to work with
mongo. i build it with debug symbols, boot up gdb mongo, and then it
doesn't let me do anything. I can't gdb mongo
jstests/aggregation/testall.js, either. I'm probably doing something wrong,
but I can't find anything on the internet on how to use mongo with gdb :(.
Should I gdb mongod? But then I can't ./mongo
jstests/aggregation/testall.js once i'm inside.

I'm really confused..can you help?

My default sanity check--just reading the code--is failing me because if
for a nonexistent field, the value is actually some form of NULL, I see no
reason why the program shouldn't work.

Thanks!

On Mon, May 7, 2012 at 3:25 PM, Chris Westin <
reply@reply.github.com

wrote:

What do you mean "it skips over it?" Is it not executing the condition?
That would be a sign of not having recompiled this file in the executable
you're running (presuming you're checking this in the debugger). Or, are
you saying the condition isn't true for non-existent fields? In that case,
what is the type of pString for a non-existent field?

Either way, something sounds fishy here. Please dig into the details and
let me know more.

Reply to this email directly or view it on GitHub:
#213 (comment)

Annie™

cwestin · 2012-05-08T12:57:09Z

Don't bother trying to run the debugger on mongo. mongo is just the client shell, which communicates with the mongod server, where everything interesting is happening. You want to run the debugger on mongod. If you're starting it from within emacs (which is the thing to do), start it via gud, that usually hooks up with the client program much more easily. I'm assuming you're using some system that's got a bit-mapped windowing system, yes? Run mongod under the debugger in one window while you connect to mongod from the mongo shell from another window.

afchin · 2012-05-08T18:03:25Z

I don't know how to use emacs; i (try to) use vim :\ I also don't know what
a bit-mapped windowing system is...I'm using ubuntu in a virtual box.

For

BSONType t = pString->getType();
const Value *pE = pString.get();

(gdb) print pE
$1 = (const class mongo::Value *) 0x968eb58
(gdb) print t
$2 = mongo::Undefined

So it looks like neither of them are NULL. Should I be checking
pString->getType() for Undefined as well as jstNULL, or just Undefined?

Thanks!
On Tue, May 8, 2012 at 5:57 AM, Chris Westin <
reply@reply.github.com

wrote:

Don't bother trying to run the debugger on mongo. mongo is just the
client shell, which communicates with the mongod server, where everything
interesting is happening. You want to run the debugger on mongod. If
you're starting it from within emacs (which is the thing to do), start it
via gud, that usually hooks up with the client program much more easily.
I'm assuming you're using some system that's got a bit-mapped windowing
system, yes? Run mongod under the debugger in one window while you connect
to mongod from the mongo shell from another window.

Reply to this email directly or view it on GitHub:
#213 (comment)

Annie™

cwestin · 2012-05-10T22:47:25Z

A bit mapped windowing system is one that uses a graphical user interface to show you multiple windows. From your earlier comments about not being able to run the debugger and the mongo shell, it sounded like you were limited to one shell (terminal) for some reason.

Yes, check for Undefined as well. I forgot that could be one of the values coming through. It looks like you added that. Is the test working as expected now?

afchin · 2012-05-10T22:53:38Z

Yes, the test works now!

On Thu, May 10, 2012 at 3:47 PM, Chris Westin <
reply@reply.github.com

wrote:

A bit mapped windowing system is one that uses a graphical user interface
to show you multiple windows. From your earlier comments about not being
able to run the debugger and the mongo shell, it sounded like you were
limited to one shell (terminal) for some reason.

Yes, check for Undefined as well. I forgot that could be one of the
values coming through. It looks like you added that. Is the test working
as expected now?

Reply to this email directly or view it on GitHub:
#213 (comment)

Annie™

benety · 2014-07-01T03:35:53Z

Hi Annie,

Do you still have any interest in continuing to work on this pull request? If you do, please sign the contributor's agreement and we'll start the review process (again).

Regards,
Ben

benety · 2014-07-01T03:36:11Z

https://jira.mongodb.org/browse/SERVER-5319

afchin · 2014-07-01T03:44:22Z

Nope, sorry..that was a really long time ago :(

On Mon, Jun 30, 2014 at 8:36 PM, Benety Goh notifications@github.com
wrote:

Hi Annie,

Do you still have any interest in continuing to work on this pull request?
If you do, please sign the contributor's agreement
http://www.10gen.com/contributor and we'll start the review process
(again).

Regards,
Ben

—
Reply to this email directly or view it on GitHub
#213 (comment).

Annie™

benety · 2014-07-01T13:35:18Z

No problem. Closing!

Timokasse · 2015-11-16T18:00:25Z

So how do we find the length of a string in MongoDB? Is there a solution now?

PSMDB-123 Handle WT backup when directoryperdb or wiredTigerDirectoryForIndexes is set

$strlen implemented

23fb383

ghost assigned cwestin Apr 24, 2012

cwestin reviewed May 2, 2012
View reviewed changes

Added a check for Undefined data type

f521059

ghost assigned RedBeard0531 Jul 12, 2012

benety changed the title ~~$strlen for aggregation framework~~ SERVER-5319 $strlen for aggregation framework Jul 1, 2014

benety closed this Jul 1, 2014

BradBarnich pushed a commit to BradBarnich/mongo that referenced this pull request Feb 6, 2019

Merge pull request mongodb#213 from denis-protyvenskyi/psmdb-123

a3eab56

PSMDB-123 Handle WT backup when directoryperdb or wiredTigerDirectoryForIndexes is set

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SERVER-5319 $strlen for aggregation framework #213

SERVER-5319 $strlen for aggregation framework #213

afchin commented Apr 24, 2012

cwestin May 2, 2012

cwestin commented May 2, 2012

afchin commented May 3, 2012

cwestin commented May 7, 2012

afchin commented May 8, 2012

cwestin commented May 8, 2012

afchin commented May 8, 2012

cwestin commented May 10, 2012

afchin commented May 10, 2012

benety commented Jul 1, 2014

benety commented Jul 1, 2014

afchin commented Jul 1, 2014

benety commented Jul 1, 2014

Timokasse commented Nov 16, 2015

SERVER-5319 $strlen for aggregation framework #213

SERVER-5319 $strlen for aggregation framework #213

Conversation

afchin commented Apr 24, 2012

cwestin May 2, 2012

Choose a reason for hiding this comment

cwestin commented May 2, 2012

afchin commented May 3, 2012

cwestin commented May 7, 2012

afchin commented May 8, 2012

cwestin commented May 8, 2012

afchin commented May 8, 2012

cwestin commented May 10, 2012

afchin commented May 10, 2012

benety commented Jul 1, 2014

benety commented Jul 1, 2014

afchin commented Jul 1, 2014

benety commented Jul 1, 2014

Timokasse commented Nov 16, 2015