Error when searching in empty degenerate subtable #124

kspangsege · 2013-08-14T14:07:19Z

NOT FIXED!

All search functions (Table::lookup(), Table::find_fist_int(), ...) will fail for an empty degenerate subtable, that is, a subtable whose 'ref' is null and whose m_columns is therefore unattached.

The same is true for all aggregate functions.

There are two possible routes to solving this problem:

The simple one: Add a check in each search and aggregate function to see if m_columns is attached, if not, just return tightdb::not_found (or whatever makes sense).
The complex one: Allow instantiation of column accessors even for a degenerate subtable.

The disadvantage of (1) is that it adds a fixed overhead to all searches and aggregates.

Also, (2) may offer other advantages than solving this particular problem.

\cc @kneth @bmunkholm @astigsen

…search happens on an empty degenerate table (unattached m_columns.)

kspangsege · 2013-08-14T14:10:07Z

This problem may be present in other functions than the mentioned search functions. Be sure to check.

kspangsege · 2013-08-14T14:45:17Z

It turns out that the same problem occurs for aggregate functions.

What about queries?

…or an empty degenerate subtable

kneth · 2013-08-16T10:49:18Z

I would prefer solution 2. Solution 1 implies that all language bindings and C++ applications must check for not_found everything a subtable is involved.

What I really want to be able to do:

t = tightdb.Table(["name", "string",
                   "children", [ "name", "string",
                                 "age",  "int"]
                   ])

t += ['Sarah', [ ['Jim', 17], ['Harry', 12] ] ]
t += ['Bill',  [ ] ] # no children
t += ['Joan',  [ ['John', 17 ] ] ]

for r in t:
    if r.children.where().name.contains("J").age.eq(17).count() > 0:
        print r.name + ": bingo"

Currently this will fail as Bill have no children (even if I write it in C++). Solution 1 implies that the if condition must be more complex (len(r.children) > 0 and must be added).

kspangsege · 2013-08-18T17:49:39Z

Kneth, my intention with (1) was not to require that applications and language bindings should check for the degenerate case. Instead I am suggesting that the core library makes this check initially, and return the appropriate value in the degenerate case.

Solution (1) and (2) would both fix the problem with no visible difference from the point of view of applications and language bindings.

After further consideration, I think we should choose option (1) because the potential benefit of (2) is not currently well understood. Later on, we may or may not choose to switch to (2).

kneth · 2013-08-18T19:03:06Z

What will the appropriate value be in solution (1)? A NULL might be easy to hide in the language bindings but would it be easy for C++ programmers?

kspangsege · 2013-08-18T20:06:35Z

I'm not sure what you mean. The appropriate return value for a degenerate
subtable for any of the affected functions is the same value as would be
returned for a non-degenerate empty subtable.

By a non-degenerate empty subtable, I mean a subtable that has a 'columns'
array but zero rows. This case is already properly handled, so the
appropriate return value is already well defined.

Note that a degenerate empty subtable, is one that does not even have a
'columns' array. This does not mean that it has no columns, only that the
'columns' array is created just-in-time, that is, when the first row is
added.

On Sun, Aug 18, 2013 at 9:03 PM, Kenneth Geisshirt <[email protected]

wrote:

What will the appropriate value be in solution (1)? A NULL might be easy
to hide in the language bindings but would it be easy for C++ programmers?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/124#issuecomment-22836760
.

kneth · 2013-08-19T06:11:36Z

It sounds like the solution I want - a (sub)table with zero rows.

👍

kspangsege · 2013-08-19T14:02:53Z

Nice thumbs-up :-)

astigsen · 2013-08-19T15:04:21Z

We also need to make sure that it works on read-only tables. Currently
pulling out an empty (non-exisiting) sub-table from a read-only table is an
error, as it will create the table.

On Mon, Aug 19, 2013 at 7:02 AM, Kristian Spangsege <
[email protected]> wrote:

Nice thumbs-up :-)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/124#issuecomment-22873759
.

kspangsege · 2013-08-19T15:24:35Z

Alexander, I doubt it, unless we violate C++ constness in our language
bindings.

In C++ there is no problem. A read transaction will only give you
const-access to the group. With that, the degenerate subtables will not get
their columns array instantiated.

If we violate that constness constraint in our language bindings, then we
have a problem. With this, and probably with other things too. If this is
the case, I think the appropriate way ahead si to modify the language
bindings so they either stop violating the constness, or takes meassures to
cancel the violations, that is, if it casts away constness at first to
store the pointer, it must then cast back the constness before calling any
Group method.

On Mon, Aug 19, 2013 at 5:04 PM, astigsen [email protected] wrote:

We also need to make sure that it works on read-only tables. Currently
pulling out an empty (non-exisiting) sub-table from a read-only table is
an
error, as it will create the table.

On Mon, Aug 19, 2013 at 7:02 AM, Kristian Spangsege <
[email protected]> wrote:

Nice thumbs-up :-)

—
Reply to this email directly or view it on GitHub<
https://github.com/Tightdb/tightdb/pull/124#issuecomment-22873759>
.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/124#issuecomment-22878495
.

kspangsege · 2013-08-19T15:31:50Z

Here is one way to go about it:

Group wrapper:
const Group* m_ro_group; // Always present
Group* m_group; // Present only in read/write mode

Now, m_group doubles as a flag for mutability, so no other flag is needed.

All invocations of read-only functions in the core library API, should
simply call via m_ro_group. No check is needed.

Mutating functions should check for the presence of m_group, and if present
call the core library via m_group.

It's simple and safe, as it requires no constness casting.

On Mon, Aug 19, 2013 at 5:24 PM, Kristian Spangsege [email protected] wrote:

Alexander, I doubt it, unless we violate C++ constness in our language
bindings.

In C++ there is no problem. A read transaction will only give you
const-access to the group. With that, the degenerate subtables will not get
their columns array instantiated.

If we violate that constness constraint in our language bindings, then we
have a problem. With this, and probably with other things too. If this is
the case, I think the appropriate way ahead si to modify the language
bindings so they either stop violating the constness, or takes meassures to
cancel the violations, that is, if it casts away constness at first to
store the pointer, it must then cast back the constness before calling any
Group method.

On Mon, Aug 19, 2013 at 5:04 PM, astigsen [email protected]:

We also need to make sure that it works on read-only tables. Currently
pulling out an empty (non-exisiting) sub-table from a read-only table is
an
error, as it will create the table.

On Mon, Aug 19, 2013 at 7:02 AM, Kristian Spangsege <
[email protected]> wrote:

Nice thumbs-up :-)

—
Reply to this email directly or view it on GitHub<
https://github.com/Tightdb/tightdb/pull/124#issuecomment-22873759>
.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/124#issuecomment-22878495
.

kspangsege · 2013-08-19T15:37:16Z

If fact, I doubt ther will be any good reason not to follow this precise
scheme in all out language bindings. For table wrappers too.

On Mon, Aug 19, 2013 at 5:31 PM, Kristian Spangsege [email protected] wrote:

Here is one way to go about it:

Group wrapper:
const Group* m_ro_group; // Always present
Group* m_group; // Present only in read/write mode

Now, m_group doubles as a flag for mutability, so no other flag is needed.

All invocations of read-only functions in the core library API, should
simply call via m_ro_group. No check is needed.

Mutating functions should check for the presence of m_group, and if
present call the core library via m_group.

It's simple and safe, as it requires no constness casting.

On Mon, Aug 19, 2013 at 5:24 PM, Kristian Spangsege [email protected]:

Alexander, I doubt it, unless we violate C++ constness in our language
bindings.

In C++ there is no problem. A read transaction will only give you
const-access to the group. With that, the degenerate subtables will not get
their columns array instantiated.

If we violate that constness constraint in our language bindings, then we
have a problem. With this, and probably with other things too. If this is
the case, I think the appropriate way ahead si to modify the language
bindings so they either stop violating the constness, or takes meassures to
cancel the violations, that is, if it casts away constness at first to
store the pointer, it must then cast back the constness before calling any
Group method.

On Mon, Aug 19, 2013 at 5:04 PM, astigsen [email protected]:

We also need to make sure that it works on read-only tables. Currently
pulling out an empty (non-exisiting) sub-table from a read-only table is
an
error, as it will create the table.

On Mon, Aug 19, 2013 at 7:02 AM, Kristian Spangsege <
[email protected]> wrote:

Nice thumbs-up :-)

—
Reply to this email directly or view it on GitHub<
https://github.com/Tightdb/tightdb/pull/124#issuecomment-22873759>
.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/124#issuecomment-22878495
.

kspangsege · 2013-09-24T18:58:08Z

Lasse, I have reassigned this one to you, as I believe Brian asked you to look at it. Please remember to commit your changes to the branch associated with this pull request.

rrrlasse · 2013-09-24T19:30:44Z

Yup, will do

On Tue, Sep 24, 2013 at 8:58 PM, Kristian Spangsege <
[email protected]> wrote:

Lasse, I have reassigned this one to you, as I believe Brian asked you to
look at it. Please remember to commit your changes to the branch associated
with this pull request.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/124#issuecomment-25032713
.

… are done for updating queries (such as delete()). Asana task has been created with this issue

…mpty-table-search-error Conflicts: .dir-locals.el test/test_table.cpp

Error when searching in empty degenerate subtable

Fsa/prerelease testing Change-Id: Ia2eb875f7c30babfde46abbde21870e04d0c8210

Reeanbling failing unit test. The reason for the failure is that the …

60be06b

…search happens on an empty degenerate table (unattached m_columns.)

ghost assigned kneth Aug 14, 2013

kspangsege mentioned this pull request Aug 14, 2013

This unit test will probably fail :-) #121

Merged

New unit test that checks many of the functions that currently fail f…

e6b4143

…or an empty degenerate subtable

Merge branch 'master' into ks-empty-table-search-error

d793277

ghost assigned rrrlasse Sep 24, 2013

rrrlasse added 5 commits September 27, 2013 12:19

Fixed those cases that were tested with unit tests

e165bba

fixed bug

acc17b0

bug

4f4fd11

queries now available in read transactions. But beware that no checks…

cae9d20

… are done for updating queries (such as delete()). Asana task has been created with this issue

Merge branch 'master' of https://github.com/Tightdb/tightdb into ks-e…

16482ff

…mpty-table-search-error Conflicts: .dir-locals.el test/test_table.cpp

rrrlasse pushed a commit that referenced this pull request Sep 27, 2013

Merge pull request #124 from kspangsege/ks-empty-table-search-error

da9f25b

Error when searching in empty degenerate subtable

rrrlasse merged commit da9f25b into realm:master Sep 27, 2013

kspangsege unassigned rrrlasse Dec 18, 2014

tgoyne pushed a commit that referenced this pull request Jul 11, 2018

Merge pull request #124 from realm/fsa/prerelease-testing

9858431

Fsa/prerelease testing Change-Id: Ia2eb875f7c30babfde46abbde21870e04d0c8210

gvravikumar mentioned this pull request Oct 30, 2023

Failure to open a Realm file on Android 14 #7037

Open

sync-by-unito bot mentioned this pull request Feb 16, 2024

App crashing when starting Realm: [realm-core-11.0.2] Assertion failed: r == 0 && "File::unlock() " ... #4777

Closed

github-actions bot locked as resolved and limited conversation to collaborators Mar 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Error when searching in empty degenerate subtable #124

Error when searching in empty degenerate subtable #124

kspangsege commented Aug 14, 2013

kspangsege commented Aug 14, 2013

kspangsege commented Aug 14, 2013

kneth commented Aug 16, 2013

kspangsege commented Aug 18, 2013

kneth commented Aug 18, 2013

kspangsege commented Aug 18, 2013

kneth commented Aug 19, 2013

kspangsege commented Aug 19, 2013

astigsen commented Aug 19, 2013

kspangsege commented Aug 19, 2013

kspangsege commented Aug 19, 2013

kspangsege commented Aug 19, 2013

kspangsege commented Sep 24, 2013

rrrlasse commented Sep 24, 2013

Error when searching in empty degenerate subtable #124

Error when searching in empty degenerate subtable #124

Conversation

kspangsege commented Aug 14, 2013

kspangsege commented Aug 14, 2013

kspangsege commented Aug 14, 2013

kneth commented Aug 16, 2013

kspangsege commented Aug 18, 2013

kneth commented Aug 18, 2013

kspangsege commented Aug 18, 2013

kneth commented Aug 19, 2013

kspangsege commented Aug 19, 2013

astigsen commented Aug 19, 2013

kspangsege commented Aug 19, 2013

kspangsege commented Aug 19, 2013

kspangsege commented Aug 19, 2013

kspangsege commented Sep 24, 2013

rrrlasse commented Sep 24, 2013