Skip to content

Primitive List feature(Part7: Schema)#5209

Merged
zaki50 merged 30 commits intofeature/primitive_listfrom
my/primitive_list_7_Schema
Sep 13, 2017
Merged

Primitive List feature(Part7: Schema)#5209
zaki50 merged 30 commits intofeature/primitive_listfrom
my/primitive_list_7_Schema

Conversation

@zaki50
Copy link
Copy Markdown
Contributor

@zaki50 zaki50 commented Sep 5, 2017

This PR is based on Part 6 (#5208)

This PR implements schema related feature required from proxy class.

@zaki50 zaki50 changed the title Primitive List feature(Part6: Schema) Primitive List feature(Part7: Schema) Sep 5, 2017
@zaki50 zaki50 added this to the 4.0 (RMP 2.0) milestone Sep 5, 2017
@zaki50 zaki50 mentioned this pull request Sep 6, 2017
21 tasks
@zaki50 zaki50 changed the base branch from my/primitive_list_6_OsList to feature/primitive_list September 11, 2017 11:11
@zaki50 zaki50 force-pushed the feature/primitive_list branch from 0fb5ed3 to afaf891 Compare September 11, 2017 11:30
OBJECT("OBJECT", "Object", false),
LIST("LIST", "List", true),

BACKLINK("LINKING_OBJECTS", null, true),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this result https://github.com/realm/realm-java/pull/5209/files#diff-36f5b86acab52385e590ad92a6d7a80cR690 add backlink RealmResults as a persisted property?
Would isList() useful for backlinks? maybe change it to isRealmList and return false for backlink if it is not useful for it?

@@ -1943,7 +1952,16 @@ private Constants.RealmFieldType getRealmType(VariableElement field) {
return Constants.RealmFieldType.OBJECT;
}
if (Utils.isRealmList(field)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems Field.isList() is not needed?

if (table->get_column_type(S(columnIndex)) != type_Table) {
return to_jbool(table->is_nullable(column_index)); // noexcept
}
return to_jbool(table->get_descriptor()->get_subdescriptor(S(columnIndex))->is_nullable(S(0))); // noexcept
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why ? would not primitive list be always nullable?

Copy link
Copy Markdown
Contributor Author

@zaki50 zaki50 Sep 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no.
nullable for primitive list means that the list can contain null as value.

So, your change in ff3c205 was not correct.
I'll fix this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

}

return static_cast<jint>(TBL(nativeTablePtr)->get_column_type(S(columnIndex))); // noexcept
auto column_type = TBL(nativeTablePtr)->get_column_type(S(columnIndex)); // noexcept
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Table.getColumnType() used anywhere? I thought we should just delete this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might use it in the Dynamic API. I didn't check though

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used in MutableRealmObjectSchema at least.

if (column_type != type_Table) {
return static_cast<jint>(column_type);
}
return static_cast<jint>(ROW(nativeRowPtr)->get_table()->get_descriptor()->get_subdescriptor(S(columnIndex))->get_column_type(S(0))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds hacky ... please add a FIXME here and i will check -- if Object Store changed the enum, we are quite difficult to figure out what is going wrong.

@zaki50 zaki50 merged commit d790a86 into feature/primitive_list Sep 13, 2017
@zaki50 zaki50 deleted the my/primitive_list_7_Schema branch September 15, 2017 05:44
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants