[DISCUSSION] Introducing a History Trait as Part of UserSession / UserQuery

[osipovartem]

✨ Motivation

The proposal is to integrate a history: Arc (WorksheetStore) field into the UserSession structure to enable first-class support for per-session query history. This will mirror behavior found in platforms like Snowflake, where functions like LAST_QUERY_ID() return query metadata for recently executed statements.

pub struct UserSession {
    pub metastore: Arc<dyn Metastore>,
    pub ctx: SessionContext,
    pub ident_normalizer: IdentNormalizer,
    pub executor: DedicatedExecutor,
    pub history: Arc<dyn HistoryStore>, // ← new field
}

The HistoryStore will be a trait responsible for recording and accessing query execution history within a session.

---

🔍 Problem with Current Architecture

Currently, history recording is handled in core-history via a custom implementation of the ExecutionService:

impl ExecutionService for RecordingExecutionService {
    async fn query(...) { ... }
}

This approach leads to duplication and architectural coupling that can be avoided. It also limits query history visibility strictly to the external recording service layer, rather than integrating it into the session-level context where it naturally belongs.

---

✅ Proposed Solution

Remove the existing RecordingExecutionService impl with a cleaner and unified CoreExecutionService implementation that records query history via the new History trait:

pub struct CoreExecutionService {
    metastore: Arc<dyn Metastore>,
    df_sessions: Arc<RwLock<HashMap<String, Arc<UserSession>>>>,
}

#[async_trait::async_trait]
impl ExecutionService for CoreExecutionService {
    async fn query(...) {
        // execute query
        // record query in session.history
    }
}

By making history part of the UserSession, we gain several benefits:
• History recording becomes mandatory and consistent across all queries.
• Query history (including last_query_id() support) is available at runtime inside SQL query processing.
• We avoid unnecessary indirection and implementation overhead in core-history.

---

🧩 Benefits
• ✅ Cleaner architecture with centralized session state.
• ✅ Full support for SQL functions like LAST_QUERY_ID().
• ✅ Removes boilerplate from RecordingExecutionService.
• ✅ Enables more extensible and testable history handling.

---

🔄 Next Steps
• Remove history recording logic from RecordingExecutionService to a part of CoreExecutor query.
• Add tests to ensure history works correctly across SQL query executions and is session-isolated.

[DanCodedThis]

pub struct CoreExecutionService {
    metastore: Arc<dyn Metastore>,
    df_sessions: Arc<RwLock<HashMap<String, Arc<UserSession>>>>,
}

#[async_trait::async_trait]
impl ExecutionService for CoreExecutionService {
    async fn query(...) {
        // execute query
        // record query in session.history
    }
}

@osipovartem Do we even need the ExecutionService trait, if it will be implemented by one thing?

[osipovartem]

Using a trait instead of a concrete struct offers several advantages, especially when designing components like a history store or session-related logic
By defining a trait, you’re decoupling the interface from the implementation. This allows multiple implementations of the same behavior, without tying the system to a specific struct.

For example, a HistoryStore trait can be implemented by an in-memory store, a database-backed store, or even a mock version for testing — all without modifying the consuming code.
For example, ExecutionService can be impelmented with support of mocks functionality where DELETE operations are supported

A trait defines a contract of what a component must do, not how it must do it. This makes the boundaries of functionality more explicit and easier to reason about.
Traits make it easy to inject custom/mock implementations in unit tests, enabling isolated and deterministic testing.

pub struct CoreExecutionService {
metastore: Arc,
df_sessions: Arc<RwLock<HashMap<String, Arc>>>,
}

#[async_trait::async_trait]
impl ExecutionService for CoreExecutionService {
async fn query(...) {
// execute query
// record query in session.history
}
}

@osipovartem Do we even need the ExecutionService trait, if it will be implemented by one thing?

[rampage644]

At this moment I begin to agree wrapping ExecutionService to extend its functionality might not be the "correct" abstraction and the split in #744 could be reversed.

I also understand @DanCodedThis suggestion - originally the trait was separated from the implementation exactly to allow another implementation that wraps the original. But Metatstore and WorksheetStore are also used as trait objects, so perhaps for consistency we might want to keep at as is or maybe reassess if we want to go forward with trait/implementation split in general.

[rampage644]

I also think there is a need to design saving query in a history a bit differently, or at least decide current implementation is satisfactory for now. As discussed in #729 original implementation relied on an UI response that affected how it was done.

[YaroslavLitvinov]

Integrating History trait into the UserSession looks natural and in fact is a logical prerequisite for implementing functions like LAST_QUERY_ID().

Also LAST_QUERY_ID will be needed a storage level support, and WorksheetsStore must expose capabilities needed for such functions to be efficiently implemented.

[osipovartem]

Next potential steps from https://github.com/Embucket/embucket/pull/951

• ExecutionService returns its own struct/models (with no dependency on serde)
• HistoryStore operates on its own structs (ideally with no serde dependency, but I realize this might be hard)
• in query() we call history store to maybe even construct query record and later persist it
• It is history store responsibily to convert to whatever format it wants data to be stored
• API handlers call history store to get query records again with query records models and can convert it to perhaps different representation. Or we refactor it later such that it calls query engine with select * from embucket.history limit 100 order by start_time desc instead of going for HistoryStore