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

Fix: Ensure iso_time uses user TZ when tz_override is empty #2106

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

mattmundell
Copy link
Contributor

@mattmundell mattmundell commented Nov 7, 2023

What

In pl/pgsql function iso_time, convert gvmd.tz_override to NULL when it is the empty string.

Why

This ensures that the user's timezone setting is used (instead of always UTC).

This has two effects:

  1. GET commands now correctly return fields like CREATION_TIME in the user's timezone.

time o m m '<get_info type="nvt" filter="sort-reverse=created rows=10 first=1000"/>' > /tmp/info
After:
<creation_time>2023-03-31T11:52:43+02:00</creation_time>
Before:
<creation_time>2023-03-31T09:52:43Z</creation_time>

  1. iso_time is much faster

SELECT iso_time (creation_time), iso_time (modification_time) FROM nvts ORDER BY creation_time DESC LIMIT 10 OFFSET 85940;
Before: 11s
After: 3s

In particular this means the GSA SecInfo > NVTs page is much faster when using the pager arrows (going to the end page was 11s and is now 4s for me).

Why is it faster? The broken coalesce was causing the empty string to be passed as zone to iso_time(seconds, zone). I think this was causing the exception to be thrown (and caught), slowing things down.

@mattmundell mattmundell requested a review from a team as a code owner November 7, 2023 15:22
Copy link

github-actions bot commented Nov 7, 2023

Conventional Commits Report

Type Number
Bug Fixes 1

🚀 Conventional commits found.

@timopollmeier timopollmeier merged commit a32b624 into greenbone:main Nov 10, 2023
9 checks passed
@mattmundell mattmundell deleted the iso-time-user-tz branch November 13, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants