Hi,
The attached patch does this:
1. Sanitize the if-else bracket style for the earlier patch.
2. Use 'IS DISTINCT FROM' instead of <> in an exclusion condition.
Two issues remain that I can't resolve:
1. Rightly as you point out thread->IsRunning() needs to be called only if
it exists. On rigorous testing the application bombs if this thread is
non-existent (backtrace attached). Any good way to check whether a thread is
running ? Any flag etc ?
2. The case of selection and deselection of the same field name can
(ideally) cause a bit of a complication.
Consider this:
a. M=10 AND M <> 10
b. M = 10 AND (N = 10 OR M <> 10)
If we are to filter out the old test condition replacing it with a new test
condition using a string parser how does it understand the difference
between case (a) and case (b) ? Unless I am missing the point, I think to
effectively resolve this would need a inverted tree structure for the where
clause which I think is a bit of an overkill for now ?
Any hints for a better way to work around these ?
*Robins*
---------- Forwarded message ----------
From: Dave Page <[EMAIL PROTECTED]>
Date: Feb 6, 2008 5:11 PM
Subject: Re: [pgadmin-hackers] Fwd: Filter by Selection on Grid
To: Robins Tharakan <[EMAIL PROTECTED]>
Cc: [email protected]
On Feb 6, 2008 11:02 AM, Robins Tharakan <[EMAIL PROTECTED]> wrote:
> > ... snip ...
> Sure ! Points that I'll take into account from now on.
>
>
> > One other thing I noticed that should be fixed in a new patch - if you
> > sort ascending on a column, and then sort descending, it will add
> > both. It should remove the ascending and replace it with the
> > descending sort.
>
> True... and one more thing that I was working on (when I responded to
> Guillaume the other day) was that if a person does an 'exclude by
selection'
> on a field with a value = 10, we simply put a [WHERE] value <> 10
filter...
> however I think this is incomplete.
>
> instead of
> WHERE value <> 10
>
> we should rather put
>
> WHERE (value <> 10 OR value IS NULL)
> Or
> WHERE (value IS DISTINCT FROM 10)
>
> This is because as per the UI, the user does not want value 10, but he
still
> would want a NULL value in the records.
Yes, thats a good point. I look forward to seeing the patch ;-)
cheers, Dave.
Index: pgadmin/frm/frmEditGrid.cpp
===================================================================
--- pgadmin/frm/frmEditGrid.cpp (revision 7060)
+++ pgadmin/frm/frmEditGrid.cpp (working copy)
@@ -477,29 +477,30 @@
size_t old_filter_string_length = GetFilter().Trim().Len();
- if (old_filter_string_length > 0) {
+ if (old_filter_string_length > 0)
new_filter_string = GetFilter().Trim() + wxT(" \n AND ");
- }
-
- if (table->IsColText(curcol)) {
- if (sqlGrid->GetCellValue(currow, curcol).IsNull()) {
+ if (table->IsColText(curcol))
+ {
+
+ if (sqlGrid->GetCellValue(currow, curcol).IsNull())
+ {
new_filter_string += column_label + wxT(" IS NULL ");
- } else {
-
- if (sqlGrid->GetCellValue(currow, curcol) == wxT("\'\'")) {
+ }
+ else
+ {
+ if (sqlGrid->GetCellValue(currow, curcol) == wxT("\'\'"))
new_filter_string += column_label + wxT(" = ''");
- } else {
+ else
new_filter_string += column_label + wxT(" = ") + connection->qtDbString(sqlGrid->GetCellValue(currow, curcol)) + wxT(" ");
- }
}
- } else {
-
- if (sqlGrid->GetCellValue(currow, curcol).IsNull()) {
+ }
+ else
+ {
+ if (sqlGrid->GetCellValue(currow, curcol).IsNull())
new_filter_string += column_label + wxT(" IS NULL ");
- } else {
+ else
new_filter_string += column_label + wxT(" = ") + sqlGrid->GetCellValue(currow, curcol);
- }
}
SetFilter(new_filter_string);
@@ -519,28 +520,29 @@
size_t old_filter_string_length = GetFilter().Trim().Len();
- if (old_filter_string_length > 0) {
+ if (old_filter_string_length > 0)
new_filter_string = GetFilter().Trim() + wxT(" \n AND ");
- }
- if (table->IsColText(curcol)) {
- if (sqlGrid->GetCellValue(currow, curcol).IsNull()) {
+ if (table->IsColText(curcol))
+ {
+ if (sqlGrid->GetCellValue(currow, curcol).IsNull())
+ {
new_filter_string += column_label + wxT(" IS NOT NULL ");
- } else {
-
- if (sqlGrid->GetCellValue(currow, curcol) == wxT("\'\'")) {
- new_filter_string += column_label + wxString::Format(_(" <> '' ")) ;
- } else {
- new_filter_string += column_label + wxT(" <> ") + connection->qtDbString(sqlGrid->GetCellValue(currow, curcol)) + wxT(" ");
- }
+ }
+ else
+ {
+ if (sqlGrid->GetCellValue(currow, curcol) == wxT("\'\'"))
+ new_filter_string += column_label + wxString::Format(_(" IS DISTINCT FROM '' ")) ;
+ else
+ new_filter_string += column_label + wxT(" IS DISTINCT FROM ") + connection->qtDbString(sqlGrid->GetCellValue(currow, curcol)) + wxT(" ");
}
- } else {
-
- if (sqlGrid->GetCellValue(currow, curcol).IsNull()) {
+ }
+ else
+ {
+ if (sqlGrid->GetCellValue(currow, curcol).IsNull())
new_filter_string += column_label + wxT(" IS NOT NULL ") ;
- } else {
- new_filter_string += column_label + wxT(" <> ") + sqlGrid->GetCellValue(currow, curcol);
- }
+ else
+ new_filter_string += column_label + wxT(" IS DISTINCT FROM ") + sqlGrid->GetCellValue(currow, curcol);
}
SetFilter(new_filter_string);
@@ -567,9 +569,8 @@
size_t old_sort_string_length = GetSortCols().Trim().Len();
- if (old_sort_string_length > 0) {
+ if (old_sort_string_length > 0)
new_sort_string = GetSortCols().Trim() + wxT(" , ");
- }
new_sort_string += column_label + wxT(" ASC ");
@@ -589,9 +590,8 @@
size_t old_sort_string_length = GetSortCols().Trim().Len();
- if (old_sort_string_length > 0) {
+ if (old_sort_string_length > 0)
new_sort_string = GetSortCols().Trim() + wxT(" , ");
- }
new_sort_string += column_label + wxT(" DESC ");
ASSERT INFO:
../include/wx/thrimpl.cpp(42): assert "m_internal" failed in Lock():
wxMutex::Lock(): not initialized
BACKTRACE:
[1] wxMutex::Lock()
[2] wxCriticalSection::Enter() /usr/include/wx-2.8/wx/thread.h:271
[3] wxCriticalSectionLocker() /usr/include/wx-2.8/wx/thread.h:287
[4] wxThread::IsRunning() cons)
[5] frmEditGrid::OnCellRightClick(wxGridEvent&)
/projects/pgadmin/pgadmin3/pgadmin/./frm/frmEditGrid.cpp:414
[6] wxAppConsole::HandleEvent(wxEvtHandler*, void (wxEvtHandler::*)(wxEvent&),
wxEvent&) cons)
[7] wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&,
wxEvtHandler*, wxEvent&)
[8] wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*)
[9] wxEvtHandler::ProcessEvent(wxEvent&)
[10] wxEvtHandler::ProcessEvent(wxEvent&)
[11] wxWindowBase::TryParent(wxEvent&)
[12] wxEvtHandler::ProcessEvent(wxEvent&)
[13] wxEvtHandler::ProcessEvent(wxEvent&)
[14] wxScrollHelperEvtHandler::ProcessEvent(wxEvent&)
[15] wxGrid::SendEvent(int, int, int, wxMouseEvent&)
[16] wxGrid::ProcessGridCellMouseEvent(wxMouseEvent&)
[17] wxGridWindow::OnMouseEvent(wxMouseEvent&)
[18] wxAppConsole::HandleEvent(wxEvtHandler*, void (wxEvtHandler::*)(wxEvent&),
wxEvent&) cons)
[19] wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&,
wxEvtHandler*, wxEvent&)
[20] wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*)
[21] wxEvtHandler::ProcessEvent(wxEvent&)
[22] wxWindow::GTKProcessEvent(wxEvent&) cons)
[23] _gtk_marshal_BOOLEAN__BOXED()
[24] g_closure_invoke()
[25] g_signal_emit_valist()
[26] g_signal_emit()
[27] gtk_propagate_event()
[28] gtk_main_do_event()
[29] g_main_context_dispatch()
[30] g_main_loop_run()
[31] gtk_main()
[32] wxEventLoop::Run()
[33] wxAppBase::MainLoop()
---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster
