Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set correct posting list type while creating it in live loader. #5012

Merged
merged 1 commit into from
Mar 25, 2020

Conversation

arijitAD
Copy link

@arijitAD arijitAD commented Mar 24, 2020

Fixes DGRAPH-1100
Posting list type was not properly set and hence it was panicking while generating tokens.
Tested this with the following data.
Data File

<_:blank> <ida> "991" .
<_:blank> <ida> "991"^^<xs:int> .
<10511170691297417216> <initial_release_date> "1957-01-01T00:00:00Z" .
<m.010rcp1x>    <initial_release_date>  "1971-06-12"^^<http://www.w3.org/2001/XMLSchema#date>

Schema File

ida: int @index(int) .
initial_release_date : datetime @index(year) .

The same code for converting the posting list is there in copyValue() in gql/mutation.go
Tested this patch in v20.03.0-beta.20200320 as well.


This change is Reviewable

@arijitAD arijitAD requested review from manishrjain and a team as code owners March 24, 2020 06:44
@arijitAD arijitAD requested review from martinmr and MichelDiz March 24, 2020 06:49
Copy link
Contributor

@martinmr martinmr left a comment

Choose a reason for hiding this comment

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

I have a question but if the answer doesn't require any code changes then this pr :lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @arijitAD, @manishrjain, and @MichelDiz)


dgraph/cmd/live/batch.go, line 304 at r1 (raw file):

			Value: de.GetValue(),
		}
		// If the value type is not already set according to the schema, set it to string and

Why is this not needed anymore?

Copy link
Author

@arijitAD arijitAD left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @manishrjain, @martinmr, and @MichelDiz)


dgraph/cmd/live/batch.go, line 304 at r1 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…

Why is this not needed anymore?

If the value type is not set according to schema then data will be set to binary type by default and the next line converts the data to the schema type.

@arijitAD arijitAD merged commit 7bed25a into master Mar 25, 2020
@arijitAD arijitAD deleted the arijitAD/liveloader-data-conversion branch March 25, 2020 10:33
arijitAD pushed a commit that referenced this pull request Apr 22, 2020
* Fix live loader data conversion for conflict key generation.

(cherry picked from commit 7bed25a)
danielmai pushed a commit that referenced this pull request Apr 22, 2020
* Fix live loader data conversion for conflict key generation.

(cherry picked from commit 7bed25a)
danielmai added a commit that referenced this pull request Apr 22, 2020
…r. (#5012)"

This reverts commit 37fa65e.

This was mistakenly cherry-picked to release/v1.2.2 instead of
release/v1.2.
danielmai pushed a commit that referenced this pull request Apr 24, 2020
* Fix live loader data conversion for conflict key generation.
danielmai pushed a commit that referenced this pull request Apr 24, 2020
* Fix live loader data conversion for conflict key generation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants