search

pdphilip / laravel-elasticsearch

Laravel Elasticsearch: An Elasticsearch implementation of Laravel's Eloquent ORM
https://elasticsearch.pdphilip.com/
MIT License
94 stars 17 forks source link

Datetime convert to unixtime #20

Closed abkrim closed 7 months ago

abkrim commented 7 months ago

I have observed some problems, as always, with the dateTime issue.

At first, I thought it was a matter of my mapping, since I don't use the default, "strict_date_optional_time||epoch_millis" but "epoch_second||yyyy-MM-dd HH:mm:ss". So, I reindexed my data locally to see if the problem persisted with the official mapping.

And yes, it persists. It has been clear to me for a long time that Elasticsearch, with its changes, sometimes complicates things a lot, and that the reality is that date queries are better done in Unix time format. Since version 6, which is the one I started working with, it has never given me any problems.

That forces me to always do the Unix time conversion myself, but with Carbo, that is not a big problem.

Proof of concept

Laravel

Before reviewing the package and trying to upload a PR with a possible solution, I performed some concept tests on my data from the last few days, which I have locally.

I did the two tests on the two mappings. The one I use and the one with the official format dateTime.

For testing, I used the model with its connections, clearing the caches just in case.

Then I ran the tests in the Kibana console of my installation (it is not the one for the package tests) and the result was the same.

I only get results using the unixtime equivalent of my query date in the query.

// Works
$analyzers = WorkAnalyzer::orderBy('datetime', 'desc')->where('status_code', '200')->whereDate('datetime', '>=', '1710892800')->count();  // result 146501 docs
// Not working
// $analyzers = WorkAnalyzer::orderBy('datetime', 'desc')->where('status_code', '200')->where('datetime', '>=', '2024-03-20')->count();  // result 0
// $analyzers = WorkAnalyzer::orderBy('datetime', 'desc')->where('status_code', '200')->where('datetime', '2024-03-20')->count(); // result 0

// $analyzers = WorkAnalyzer::orderBy('datetime', 'desc')->where('status_code', '200')->whereDate('datetime', '>=', '2024-03-20')->count(); // result 0

Kibana

GET /work-analyzers/_mapping/field/datetime
{
  "analyzers-2022111701": {
    "mappings": {
      "datetime": {
        "full_name": "datetime",
        "mapping": {
          "datetime": {
            "type": "date",
            "format": "epoch_second||yyyy-MM-dd HH:mm:ss",
            "ignore_malformed": true
          }
        }
      }
    }
  }
}

GET /analyzers-2024032301/_mapping/field/datetime
{
  "analyzers-2024032301": {
    "mappings": {
      "datetime": {
        "full_name": "datetime",
        "mapping": {
          "datetime": {
            "type": "date",
            "ignore_malformed": true
          }
        }
      }
    }
  }
}
// 1710892800 = 2024-03-20T00:00:00Z
GET /analyzers-2024032301/_count
{
    "query": {
        "bool": {
            "filter": [
                { "term":  { "status_code": "200" }},
                { "range": { "datetime": { "gte": "1710892800" }}}
            ]
        }
    }
}
{
  "count": 146501,
  "_shards": {
    "total": 1,
    "successful": 1,
    "skipped": 0,
    "failed": 0
  }
}

GET /analyzers-2024032301/_count
{
    "query": {
        "bool": {
            "filter": [
                { "term":  { "status_code": "200" }},
                { "range": { "datetime": { "gte": "2024-03-20T00:00:00Z"}}}
            ]
        }
    }
}
{
  "count": 0,
  "_shards": {
    "total": 1,
    "successful": 1,
    "skipped": 0,
    "failed": 0
  }
}
GET /analyzers-2024032301/_count
{
    "query": {
        "bool": {
            "filter": [
                { "term":  { "status_code": "200" }},
                { "range": { "datetime": { "gte": "2024-03-20"}}}
            ]
        }
    }
}
{
  "count": 0,
  "_shards": {
    "total": 1,
    "successful": 1,
    "skipped": 0,
    "failed": 0
  }
}

This question, very important for me, since I have hundreds of millions of documents in my indexes, and the importance of their data over time is very necessary, since it is about control of electrical consumption in municipalities, has led me for a long time to use timestamp. When everything worked, it stopped working, and the theme is always in the datetime field.

I looked for help on Elasticsearch, and etc., and in the end, I chose to always convert the queries to timestamp.

Testing

I have tried to understand how your testing works, to create a file of tests related to this issue.

But it has been impossible for me.

I can't find how you load the docs, nor do I know the operation since when it tries to run a test within QueriesTest, it forces me to run the entire eloquent group. I imagine it's because of the load.

In my case, what I do is refresh the indices in each block of tests, which allows me to treat each test as individual. Maybe a little slower.

Also, many times, when running tests, I get different results. Sometimes some tests fail, sometimes not. And after running several tests, I need to rebuild composer with --no-cache to get the tests OK.

I feel quite useless, with this topic. But hey, I prefer to leave what is highlighted. In the tests that I do in my app, I have created a battery for whereDate and for Between (I didn't do anything here, but I wanted to check that the problem was the same. Either it passes Unix time or it fails).

pdphilip commented 7 months ago

Hey @abkrim, appreciate the detailed PR. Thank you.

Few comments, as far as I can tell this result is expected. If your mapping is epoch_second||yyyy-MM-dd HH:mm:ss then ES is expecting a timestamp or date with time. Passing a date only will fail the mapping.

If you try:

WorkAnalyzer::where('status_code', '200')->where('datetime', '>=', '2024-03-20 00:00:00')->count();

I would expect that to work, else you can update your mapping to include dates only, ex:

Schema::create('work_analyzers', function (IndexBlueprint $index) {
    $index->field('date', 'datetime', [
        'format'  => 'epoch_second||yyyy-MM-dd HH:mm:ss||yyyy-MM-dd',
        'ignore_malformed' => true,
    ]);
});

With that mapping, I would expect your original query to work (let me know!)

I'm wary to try intercept and transform the date values on the query as this may have knock-on effects if there are other date mappings users are trying to pursue. It's hard to test all the possible date-related options with this.

Try my suggestions above and let me know, then let's take it from there.

PS, on the tests. They sometimes fail mostly because they save without refreshing and the data hasn't caught up yet. I need to update those tests still

abkrim commented 7 months ago

Hello.

I think I don't express myself well.

But no problem, step by step, and I'm sorry if my English is a bit cryptic. I'm not good with languages.

Step by Step new index with data for test

Create index with datetime by default in ElasticSearch 8.12

{
  "analyzers-2024032301": {
    "mappings": {
      "datetime": {
        "full_name": "datetime",
        "mapping": {
          "datetime": {
            "type": "date",
            "ignore_malformed": true
          }
        }
      }
    }
  }
}

Change in my local app $index


<?php

namespace App\Models;

use Illuminate\Support\Carbon;
use PDPhilip\Elasticsearch\Eloquent\Model;

class WorkAnalyzer extends Model
{
    public $timestamps = false;

    protected $connection = 'elasticsearch';

    //protected $index = 'work-analyzers';

    protected $index = 'analyzers-2024032301';
...
}

Run test in command artisan

public function handle()
    {
        // Funciona
        $analyzers = WorkAnalyzer::orderBy('datetime', 'desc')->where('status_code', '200')->whereDate('datetime', '>=', '1710892800')->count();
        ray($analyzers);
        // Not working
        $analyzers = WorkAnalyzer::where('status_code', '200')->where('datetime', '>=', '2024-03-20 00:00:00')->count();
        ray($analyzers);

Fail: "Failed to parse query [((status_code:\"200\") AND (datetime:[2024-03-20 00:00:00 TO *]))]", with reason: "Cannot parse '((status_code:\"200\") AND (datetime:[2024-03-20 00:00:00 TO *]))': Encountered \" <RANGE_GOOP> \"00:00:00 \"\" at line 1, column 47.\nWas expecting:\n \"TO\" ...\n "

Migration

According to your manual create a migration for a new index, but error in $index->field

<?php

use Illuminate\Database\Migrations\Migration;
use PDPhilip\Elasticsearch\Schema\Schema;
use PDPhilip\Elasticsearch\Schema\IndexBlueprint;
use PDPhilip\Elasticsearch\Schema\AnalyzerBlueprint;

return new class extends Migration
{
    /**
     * Run the migrations.
     */
    public function up(): void
    {
        Schema::create('work_analyzers2', function (IndexBlueprint $index) {
            $index->field('date', 'datetime', [
                'format' => 'epoch_second||yyyy-MM-dd HH:mm:ss||yyyy-MM-dd',
                'ignore_malformed' => true,
            ]);
        });
    }

 Error

 Call to undefined method PDPhilip\Elasticsearch\Schema\IndexBlueprint::field()

Migration with my method

After fail migration use my owned system and reindex my work-analyzers onto analyzers-2024032501 with mapping proposal.

Index analyzers-2024032501 created
{
    "acknowledged": true,
    "shards_acknowledged": true,
    "index": "analyzers-2024032501"
}
Reindexing from analyzers-2022111701 to elk/mappings/analyzers-2024032501.php
Reindex end from analyzers-2022111701 to elk/mappings/analyzers-2024032501.php

GET /analyzers-2024032501/_mapping/field/datetime

{
  "analyzers-2024032501": {
    "mappings": {
      "datetime": {
        "full_name": "datetime",
        "mapping": {
          "datetime": {
            "type": "date",
            "format": "epoch_second||yyyy-MM-dd HH:mm:ss||yyyy-MM-dd",
            "ignore_malformed": true
          }
        }
      }
    }
  }
}

Run the tests again

// Working
$analyzers = WorkAnalyzer::orderBy('datetime', 'desc')->where('status_code', '200')->whereDate('datetime', '>=', '1710892800')->count();
        ray($analyzers);
$analyzers = WorkAnalyzer::orderBy('datetime', 'desc')->where('status_code', '200')->whereBetween('datetime',['1710892800','1711355774'])->count();
        ray($analyzers);       

// Not working
$analyzers = WorkAnalyzer::where('status_code', '200')->where('datetime', '>=', '2024-03-20 00:00:00')->count();
        ray($analyzers);

Now... if not a silent return count = 0. Now error.

"reason": "Failed to parse query [((status_code:\"200\") AND (datetime:[2024-03-20 00:00:00 TO *]))]", 

{
    "error": {
        "root_cause": [
            {
                "type": "query_shard_exception",
                "reason": "Failed to parse query [((status_code:\"200\") AND (datetime:[2024-03-20 00:00:00 TO *]))]",
                "index_uuid": "iX3U5nyKQEiW-Jr5e6mZWQ",
                "index": "analyzers-2024032501"
            }
        ],
        "type": "search_phase_execution_exception",
        "reason": "all shards failed",
        "phase": "query",
        "grouped": true,
        "failed_shards": [
            {
                "shard": 0,
                "index": "analyzers-2024032501",
                "node": "dTTpCTBqRbSYbWfW4vyFUg",
                "reason": {
                    "type": "query_shard_exception",
                    "reason": "Failed to parse query [((status_code:\"200\") AND (datetime:[2024-03-20 00:00:00 TO *]))]",
                    "index_uuid": "iX3U5nyKQEiW-Jr5e6mZWQ",
                    "index": "analyzers-2024032501",
                    "caused_by": {
                        "type": "parse_exception",
                        "reason": "Cannot parse '((status_code:\"200\") AND (datetime:[2024-03-20 00:00:00 TO *]))': Encountered \" <RANGE_GOOP> \"00:00:00 \"\" at line 1, column 47.\nWas expecting:\n    \"TO\" ...\n    ",
                        "caused_by": {
                            "type": "parse_exception",
                            "reason": "Encountered \" <RANGE_GOOP> \"00:00:00 \"\" at line 1, column 47.\nWas expecting:\n    \"TO\" ...\n    "
                        }
                    }
                }
            }
        ]
    },
    "status": 400
}

The truth is that always, since I have known Elasticsearch, dates have been a tremendous headache in all versions.

That's why I have always used unixtime in searches, because in my tests I found problems with each version change.

I have no problem, I adapt my logic to the construction or need.

I also found the issue of data validation complicated, which is why in my projects with ElasticSearch I use a helper for this, and I test them intensively.

I won't waste your time anymore.

With a little time I will better understand your data loading system in tests, and I will be able to use it more properly.

I appreciate your time. Best regards

pdphilip commented 7 months ago

Very useful. Based on the errors, it looks like it's pointing to how the bridge is constructing the query string - which I've completely rewritten in v3 to avoid issues like this.

What version of the package are you using? Also which version of ES and Laravel?

abkrim commented 7 months ago

This project has not been upgraded to L11. (still...)

pdphilip commented 7 months ago

Ok great. Can you try the dev branch: https://github.com/pdphilip/laravel-elasticsearch/tree/dev

This is v3 which will be launched and maintained going forward (within the next few days). The bridge has been completely rewritten to use DSL queries almost exclusively.

I'd like to see if the same issues persist with this rebuild. And if so, address them.

Also, features like this will be there:

$index->field('date', 'datetime', [
        'format' => 'epoch_second||yyyy-MM-dd HH:mm:ss||yyyy-MM-dd',
        'ignore_malformed' => true,
]);

Please let me know & thanks for your contribution.

Laravel 11 will come with the v3 release by the way.

pdphilip commented 7 months ago

Sorry, use this for Laravel 10: https://github.com/pdphilip/laravel-elasticsearch/tree/dev-l10

abkrim commented 7 months ago

Great jobs.

Work fine in my app with Laravel 10.

Reindex with 'format' => 'epoch_second||yyyy-MM-dd HH:mm:ss||yyyy-MM-dd', and work fine, "YYYY-MM-DD", "YYYY-MM-DD HH:MM:SS", "2024-03-20T00:00:00Z"...

I will wait until the release of the 3rd for the app in production, but I will continue reading the code in my free time to try to learn more about it.

Thanks a lot.

pdphilip commented 7 months ago

Hey @abkrim - V3 has been released.

Seeing this PR is not required in the new release I'll close it down. Thanks again for all your input.