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

Filter _between is not returning correct results on uploaded_on field for uploaded files #16336

Open
TotalLag opened this issue Nov 7, 2022 · 5 comments

Comments

@TotalLag
Copy link

TotalLag commented Nov 7, 2022

Describe the Bug

Both my modified_on field and upload_on field have roughly the same timestamps (+/- a few seconds or minutes) and I'll count them as the same for this example.

I can filter on the following and get results:

"modified_on": {
      "_between": ["2022-11-05 12:00:00", "now"]
    },

However, when I replace it with "uploaded_on" I don't get any results. This is also reflected in the UI via the search bar.

To Reproduce

  1. Upload some files between different times
  2. Go to the File Library and filter based on "Modified On Is Between" some date and "now"
  3. Change the "Modified On" to "Uploaded On"
  4. Expect: Same results because timestamps are the same.
  5. Actual: No results

Errors Shown

No response

What version of Directus are you using?

9.20.4

What version of Node.js are you using?

18

What database are you using?

sqlite

What browser are you using?

msedge

How are you deploying Directus?

official container

@azrikahar
Copy link
Contributor

This seems to be specific to SQLite, as I was not able to reproduce this on PostgreSQL. Additional findings:

When we look at the uploaded_on and modified_on column on SQLite, we can observe that they are stored in different format:

Whereas PostgreSQL stores them in the same format (likely why not reproducible on PostgreSQL):

when we inspect the SQL query formed during _between filters on SQLite, we can see it is using unix timestamp:

select 
`directus_files`.`id`, `directus_files`.`modified_on`, `directus_files`.`type`, `directus_files`.`title`, `directus_files`.`filesize`
from `directus_files`
where ((`directus_files`.`uploaded_on` between ? and ?) and (`directus_files`.`type` is not null and `directus_files`.`folder` is null))
order by `directus_files`.`uploaded_on` desc 
limit ?
[1667750400000, 1667836800000, 25]

which might be why it worked on modified_on (stored in unix timestamp format) but not on uploaded_on (stored in ISO-8601 format).

This is likely because modified_on has the special date-updated:

- field: modified_on
interface: datetime
special:
- date-updated
width: half
display: datetime
readonly: true

but uploaded_on doesn't seem to have special such as date-created:

- field: uploaded_on
display: datetime
width: half
hidden: true

I then tested adding the following special to uploaded_on:

    special:
      - date-created

So now when we upload a new file, uploaded_on is saved in unix timestamp format:

The filter then seems to work properly for SQLite afterwards.

However not 100% sure if this is the correct fix or the actual root cause (needs more testing with other non directus_files fields and datetime/timestamp without special flags on SQLite).

@TotalLag
Copy link
Author

nice work! thank you for looking into this. if this is affecting sqlite, it makes me wonder what other db types are affected by this.

what's the story behind "special" is it the same as this here? https://docs.directus.io/reference/system/fields.html

i'm still new to the codebase and can't find more details on what options are available for it.

@azrikahar
Copy link
Contributor

if this is affecting sqlite, it makes me wonder what other db types are affected by this.

IIRC other db types has been resolved in past (unless you meant db vendors), so this should just be something that somehow flew under the radar for SQLite 👍 especially since both of the fields have the same timestamp type when being seeded:

uploaded_on:
type: timestamp

modified_on:
type: timestamp

what's the story behind "special" is it the same as this here? https://docs.directus.io/reference/system/fields.html

Yeah that's correct!

i'm still new to the codebase and can't find more details on what options are available for it.

Generally it shouldn't be something to be tinkered by a user, so there isn't much explicit detail on it in the docs (but could be a good thing to include nonetheless!). Ideally Directus will handle it seamlessly for you so there shouldn't be a need to modify/know them, but just to share more info in terms of codebase, they are primarily being "processed" (or cast/transform) over here:

public transformers: Transformers = {
async hash({ action, value }) {
if (!value) return;
if (action === 'create' || action === 'update') {
return await generateHash(String(value));
}
return value;
},
async uuid({ action, value }) {
if (action === 'create' && !value) {
return uuid();
}
return value;
},
async 'cast-boolean'({ action, value }) {
if (action === 'read') {
if (value === true || value === 1 || value === '1') {
return true;
} else if (value === false || value === 0 || value === '0') {
return false;
} else if (value === null || value === '') {
return null;
}
}
return value;
},
async 'cast-json'({ action, value }) {
if (action === 'read') {
if (typeof value === 'string') {
try {
return parseJSON(value);
} catch {
return value;
}
}
}
return value;
},
async conceal({ action, value }) {
if (action === 'read') return value ? '**********' : null;
return value;
},
async 'user-created'({ action, value, accountability }) {
if (action === 'create') return accountability?.user || null;
return value;
},
async 'user-updated'({ action, value, accountability }) {
if (action === 'update') return accountability?.user || null;
return value;
},
async 'role-created'({ action, value, accountability }) {
if (action === 'create') return accountability?.role || null;
return value;
},
async 'role-updated'({ action, value, accountability }) {
if (action === 'update') return accountability?.role || null;
return value;
},
async 'date-created'({ action, value, helpers }) {
if (action === 'create') return new Date(helpers.date.writeTimestamp(new Date().toISOString()));
return value;
},
async 'date-updated'({ action, value, helpers }) {
if (action === 'update') return new Date(helpers.date.writeTimestamp(new Date().toISOString()));
return value;
},
async 'cast-csv'({ action, value }) {
if (Array.isArray(value) === false && typeof value !== 'string') return;
if (action === 'read') {
if (Array.isArray(value)) return value;
if (value === '') return [];
return value.split(',');
}
if (Array.isArray(value)) {
return value.join(',');
}
return value;
},
};

@rijkvanzanten
Copy link
Member

Linear: ENG-143

@mahendraHegde
Copy link
Contributor

mahendraHegde commented Dec 28, 2023

Here's what i noticed,

both modified on and uploaded on are datetime withCURRENT_TIMESTAMP as default
Screenshot 2023-12-27 at 11 01 01 PM

When i tried inserting manually insert into `directus_files` (`filename_download`, `id`, `storage`, `title`, `type`, `uploaded_by`) values (1, 1, 1, 1, 1, 1) i noticed by default sqlite uses YYYY-MM-DD HH:MM:SS format
Screenshot 2023-12-27 at 11 02 37 PM

Fields without special are not inserted/updated from app, hence for uploaded_on sqlite default rule is kicking in and being inserted as above mentioned format.

Sqlite doesnt enforce any specific type on datetime applications can insert in one of the allowed formats
Screenshot 2023-12-27 at 11 10 16 PM

so, 1 of these 3 approaches is what we probably can take

  1. as mentioned above, use special modifier on field and migrate existing data.
  2. format the datetime selected by user to to YYYY-MM-DD HH:MM:SS format during building where clause using date helpers(
    override parse(date: string | Date): string {
    if (!date) {
    return date;
    }
    // Date generated from NOW()
    if (date instanceof Date) {
    return String(date.getTime());
    }
    // Return the time as string
    if (date.length <= 8 && date.includes(':')) {
    return date;
    }
    // Return dates in epoch milliseconds
    return String(new Date(date).getTime());
    ) based on special value or using transformers
  3. similar to approach 2, but using date functions while building query

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📋 Backlog
Development

No branches or pull requests

4 participants