Skip to content

temporal: prohibit writing map timestamps across mapsets#7211

Open
Dasux wants to merge 2 commits intoOSGeo:mainfrom
Dasux:fix-3394-cross-mapset-timestamp
Open

temporal: prohibit writing map timestamps across mapsets#7211
Dasux wants to merge 2 commits intoOSGeo:mainfrom
Dasux:fix-3394-cross-mapset-timestamp

Conversation

@Dasux
Copy link
Copy Markdown
Contributor

@Dasux Dasux commented Mar 22, 2026

This PR addresses issue #3394

The problem was when registering a map like lakes@PERMANENT , the temporal framework attempts to write timestamp data into the current mapset. This created an invalid map entry e.g, lakes@to_verify without the actual vector data.

image

The reason was ... the function write_timestamp_to_grass() is called without verifying that the map belongs to the current mapset.

Fix made:

Added an additional condition in the function update_absolute_time()... to verify if map is in its own mapset or not.

if (
                get_enable_timestamp_write()
                and self.get_mapset() == get_current_mapset()
            ):
                self.write_timestamp_to_grass()
            else:
                self.msgr.warning(
                    f"Skipping timestamp write for {self.get_map_id()} (different mapset)"
                )

Additional note:

I found multiple instances of write_timestamp_to_grass ... I've added the condition only in one of the methods... I wanted to confirm if this was the intended change required before proceeding with the other methods...

image

@github-actions github-actions bot added Python Related code is in Python libraries labels Mar 22, 2026
@ninsbl ninsbl changed the title Fix 3394 cross mapset timestamp temporal: prohibit writing map timestamps across mapsets Mar 22, 2026
Copy link
Copy Markdown
Member

@ninsbl ninsbl left a comment

Choose a reason for hiding this comment

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

Looks good to me. Have not tested if it completely fixes the issue. But it is an improvement in any case...

@Dasux
Copy link
Copy Markdown
Contributor Author

Dasux commented Mar 23, 2026

Thanks for the feedback,

currently it does not work as intended... lakes@to_verify still shows up in the vector list .... since i only applied the condition to one method//

I'll get back to you after i append the changes to the other methods

@wenzeslaus
Copy link
Copy Markdown
Member

I think you should set up a pytest-based test for this so that it clear when writing is working and when is is (intentionally) blocked.

@Dasux
Copy link
Copy Markdown
Contributor Author

Dasux commented Mar 26, 2026

I think you should set up a pytest-based test for this so that it clear when writing is working and when is is (intentionally) blocked.

@wenzeslaus --- ill look into it...
also @ninsbl ....
I just wanted to confirm---- is the bug limited to only vector maps?
Because I recently tested on a raster map --- and I found the same issue.

My initial guess, about changing the logic when write_timestamp_to_grass() calls wasn't enough. The bug still exists.
I changed the logic for two functions inside abstract_map_dataset.py which i thought were responsible for calling timestamps.

One thing to note tho:
r.info does NOT show the timestamp being written, but r.timestamp does. This is an inconsistency, and we can look into it later. Maybe in another PR.

I think the pipeline is something like this:
t.register -> update_absolute_time() (or) update_relative_time() -> write_timestamp_to_grass() -> some C library

It seems the C layer correctly enforces mapset ownership and rejects writes when the map is not in the current mapset. However, the Python layer may not be properly handling this failure, leading to inconsistent behavior.

I'm still investigating to pinpoint where the mismatch happens.

I'll setup a pytest-based test to verify the blocked and allowed test cases.

@ninsbl
Copy link
Copy Markdown
Member

ninsbl commented Mar 27, 2026

Not quite sure what is going on here. I did not dig deeper into the issue. It only occurrs with vector maps (though raster timestamps may be written to the GRASS DB, I hdid not check that).
Maybe this gives you some more clues: https://github.com/OSGeo/grass/blob/main/python/grass/temporal/c_libraries_interface.py#L1907
You may have to compare the C functions to find the reason, but a fix may well be appropriate in the Python code...
The reason I discovered the issue that a vector map appears in the current mapset after registering and cannot be deleted aftwerward.

A test would be great!

@wenzeslaus
Copy link
Copy Markdown
Member

r.info vs r.timestamp: yes separate issue

raster vs vector: Probably both, again, a test will clarify for everyone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libraries Python Related code is in Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants