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.

Sign up for GitHub

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

Jump to bottom

Implement EZP-24270: Indexable Date field type #1234

Merged
merged 2 commits into from May 5, 2015
Merged

Implement EZP-24270: Indexable Date field type #1234

merged 2 commits into from May 5, 2015

Conversation

pspanja
Copy link
Contributor

@pspanja pspanja commented Apr 17, 2015

This PR resolves https://jira.ez.no/browse/EZP-24270

A subtask of https://jira.ez.no/browse/EZP-24232

This implements Indexable definition for Date field type. Date value is indexed as integer (timestamp with time of day at 0).

@pspanja pspanja changed the title Implement EZP-24270: Indexable Time field type Implement EZP-24270: Indexable Date field type Apr 17, 2015
andrerom
andrerom reviewed Apr 19, 2015
View reviewed changes
eZ/Publish/Core/FieldType/Date/SearchField.php
new Search\Field(
'value',
$field->value->data["timestamp"],
new Search\FieldType\IntegerField()
Copy link
Contributor

Choose a reason for hiding this comment

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

ok for now I guess, but both Solr and Elastic search has native Date/DateTime types right?

@andrerom
Copy link
Contributor

+1

1 similar comment
@glye
Copy link
Member

glye commented Apr 27, 2015

+1

@pspanja pspanja force-pushed the impl-EZP-24270-indexable-date branch from 36ab4d1 to 20f7e59 Compare May 4, 2015 09:36
@pspanja
Copy link
Contributor Author

pspanja commented May 4, 2015

Now updated to index Date value in date field, as beginning of the day UTC.
This changes target value to eg. Y-m-d (or other supported date format) instead of a timestamp integer.

@pspanja pspanja force-pushed the impl-EZP-24270-indexable-date branch 2 times, most recently from 0d73afc to 402bf2d Compare May 4, 2015 13:47
@pspanja
Copy link
Contributor Author

pspanja commented May 4, 2015

Fixed target value for Solr in e88350f. This now requires the value in the full format YYYY-MM-DDThh:mm:ssZ.

@andrerom
Copy link
Contributor

andrerom commented May 4, 2015

ok, any downsides or alternatives?

@pspanja
Copy link
Contributor Author

pspanja commented May 5, 2015

The only downside is we now have to provide a date string in the full format. That is just for Solr, Elasticsearch is a more flexible and can accept Y-m-d. But it seems Solr 5 would be able to accept that as well. I'll test it out.

One alternative would be to format the value for the targeted field. It would be possible as we have all information from the Indexable definition. But I'm not convinced it would be a good idea.

andrerom
andrerom reviewed May 5, 2015
View reviewed changes
eZ/Publish/API/Repository/Tests/FieldType/DateIntegrationTest.php

protected function getSearchTargetValueOne()
{
// Handling Legacy Search Engine, which stores Checkbox value as integer
Copy link
Contributor

Choose a reason for hiding this comment

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

checkbox, same below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in af727a7.

@pspanja pspanja mentioned this pull request May 5, 2015
Merged
@pspanja pspanja force-pushed the impl-EZP-24270-indexable-date branch from af727a7 to 68e534b Compare May 5, 2015 09:23
@pspanja
Copy link
Contributor Author

pspanja commented May 5, 2015

But it seems Solr 5 would be able to accept that as well.

It is actually supported only by DateRangeField, which we don't use. I going to merge this as is and we can document on the field type how to target the field.

@pspanja pspanja force-pushed the impl-EZP-24270-indexable-date branch from 68e534b to 09271fc Compare May 5, 2015 11:51
pspanja added a commit that referenced this pull request May 5, 2015
@pspanja
Merge pull request #1234 from ezsystems/impl-EZP-24270-indexable-date
c0cbeca 
Implement EZP-24270: Indexable Date field type
@pspanja pspanja merged commit c0cbeca into master May 5, 2015
@pspanja pspanja deleted the impl-EZP-24270-indexable-date branch May 5, 2015 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Ready for review
Milestone
No milestone
3 participants
@pspanja @andrerom @glye