Hi Thava, Ingo,
Thava proposes a solution where possible warnings generated when
execute_direct() is used will not be shown to user if thd->killed is set.
I think that such warnings (other than ER_QUERY_INTERRUPTED) should be shown
to the user. Thava thinks it is acceptable that they are ignored.
Ingo: could you state your position here. If you agree with Thava, I'll
approve this as well.
Rafal
Thava Alagu wrote:
>> COMMENT
>> -------
>> I am not convinced by your argumentation that reporting additional
>> errors/warnings after query interruption is not needed or even
>> confusing. If a user hits Ctrl+C to interrupt an operation, depending
>> on the operation a complex shutdown sequence might be executed. During
>> this shutdown sequence errors might happen or warnings be generated
>> and I think the user should see them. This might require more work on
>> our side but is not impossible to implement. E.g., instead of calling
>> copy_warnings() do an explicit loop iterating over the warnings in
>> ed_connection and add only these which are different from
>> ER_QUERY_INTERRUPTED.
>>
> My initial impression about this is that it is unnecessary. We run
> service interface
> queries mainly in the initial stages to obtain the metadata information.
> When these queries are interrupted, we already give high level warning
> information about what stage the interruption has occured.
> IMHO, that should be sufficient. Ofcourse, if there is
> software bug, these low level warnings will be useful for
> troubleshooting,
> but that is the job for the debug trace not user visible warnings.
> I am bit concerned about raising false alarms for the user who can not
> make much out of the low level warnings.
>
> Having said that I have not looked at many real test cases where
> these
> low level warnings will be really useful or raising unnecessary alarms.
> So it is possible I am overlooking few possibilities.
> If we do decide to forward the lowlevel warnings to user, this will need
> extending ed_connection with new method like append_unique_warnings()
> May be part of another patch? Let us discuss if this is needed.
>
