Re: [Postgres-xc-developers] Fix for test case xc_misc

[Ashutosh B. <ash...@en...>]

This looks good, except a cosmetic change.
The condition here is a && (b || c) && d. When spread across multiple
lines, it should be properly indented like
a &&
(b ||
   c) &&
d

so that AND/OR hierarchy is easily readable.


On Thu, May 9, 2013 at 4:35 PM, Abbas Butt <abb...@en...>wrote:

> PFA the updated patch.
>
>
> On Tue, May 7, 2013 at 2:57 PM, Ashutosh Bapat <
> ash...@en...> wrote:
>
>> Hi Abbas,
>> The right fix is at line 402. Because of the condition there, we do not
>> call GetPreferredReplicationNode() if there is FOR SHARE/FOR UPDATE clause
>> there. I think we need to modify the condition.
>>
>>  392     if (exec_nodes && exec_nodes->nodeList)
>>  393     {
>>  394         /*
>>  395          * If this is the highest level query in the query tree and
>>  396          * relations involved in the query are such that ultimate
>> JOIN is
>>  397          * replicated JOIN, choose only one of them.
>>  398          * If we do this for lower level queries in query tree, we
>> might loose
>>  399          * chance because common nodes are left out.
>>  400          */
>>  401         if (IsExecNodesReplicated(exec_nodes) &&
>>  402             exec_nodes->accesstype == RELATION_ACCESS_READ &&
>>  403             sc_context->sc_query_level == 0)
>>  404
>>  405         {
>>  406             List *tmp_list = exec_nodes->nodeList;
>>  407             exec_nodes->nodeList =
>> GetPreferredReplicationNode(exec_nodes->nodeList);
>>  408             list_free(tmp_list);
>>  409         }
>>  410         return exec_nodes;
>>  411     }
>>
>> Regarding other places where you have used the wrapper around
>> GetPreferredReplicationNode(); we don't need those code changes. We set
>> en_expr only for distributed relations and en_relid is set and used
>> properly only for DMLs when this problem would not happen. May be we should
>> add assertions at these places to make sure that we don't come there for
>> replicated tables and FOR SHARE/UDPATE SELECTs.
>>
>>
>>
>> On Tue, May 7, 2013 at 10:21 AM, Abbas Butt <abb...@en...>wrote:
>>
>>> I changed to code by creating another function and calling it right
>>> after GetRelationNodes where needed. Updated patch attached.
>>>
>>>
>>> On Tue, Apr 30, 2013 at 3:41 PM, Ashutosh Bapat <
>>> ash...@en...> wrote:
>>>
>>>> Hi Abbas,
>>>> Instead of fixing this in GetRelationNodes, we should fix it outside,
>>>> by calling GetPrefferedNode on the result returned by GetRelationNodes for
>>>> replicated tables. Please send a revised patch with this change. May be you
>>>> want to examine all callers of GetRelationNodes.
>>>>
>>>>
>>>> On Wed, Apr 24, 2013 at 10:58 PM, Abbas Butt <
>>>> abb...@en...> wrote:
>>>>
>>>>> Hi,
>>>>> The test case fails if there is no primary node in the cluster. The
>>>>> following test case was producing error.
>>>>>
>>>>> CREATE TABLE tt_22 (a int, b int) distribute by replication;
>>>>> INSERT INTO tt_22 VALUES (10);
>>>>>
>>>>> In a four datanode cluster the following query plan was being produced.
>>>>>
>>>>> explain verbose SELECT * FROM tt_22 FOR UPDATE;
>>>>>                                  QUERY
>>>>> PLAN
>>>>>
>>>>> ----------------------------------------------------------------------------
>>>>>  Data Node Scan on "__REMOTE_FQS_QUERY__"  (cost=0.00..0.00 rows=0
>>>>> width=0)
>>>>>    Output: tt_22.a, tt_22.b
>>>>>    Node/s: data_node_1, data_node_2, data_node_3, data_node_4
>>>>>    Remote query: SELECT a, b FROM tt_22 FOR UPDATE OF tt_22
>>>>> (4 rows)
>>>>>
>>>>> The reason was a missing else case in GetRelationNodes.
>>>>>
>>>>>
>>>>> --
>>>>> *Abbas*
>>>>>  Architect
>>>>>
>>>>> Ph: 92.334.5100153
>>>>>  Skype ID: gabbasb
>>>>> www.enterprisedb.co <http://www.enterprisedb.com/>m<http://www.enterprisedb.com/>
>>>>> *
>>>>> Follow us on Twitter*
>>>>> @EnterpriseDB
>>>>>
>>>>> Visit EnterpriseDB for tutorials, webinars, whitepapers<http://www.enterprisedb.com/resources-community>and more<http://www.enterprisedb.com/resources-community>
>>>>>
>>>>>
>>>>> ------------------------------------------------------------------------------
>>>>> Try New Relic Now & We'll Send You this Cool Shirt
>>>>> New Relic is the only SaaS-based application performance monitoring
>>>>> service
>>>>> that delivers powerful full stack analytics. Optimize and monitor your
>>>>> browser, app, & servers with just a few lines of code. Try New Relic
>>>>> and get this awesome Nerd Life shirt!
>>>>> http://p.sf.net/sfu/newrelic_d2d_apr
>>>>> _______________________________________________
>>>>> Postgres-xc-developers mailing list
>>>>> Pos...@li...
>>>>> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Best Wishes,
>>>> Ashutosh Bapat
>>>> EntepriseDB Corporation
>>>> The Postgres Database Company
>>>>
>>>
>>>
>>>
>>> --
>>> --
>>> *Abbas*
>>>  Architect
>>>
>>> Ph: 92.334.5100153
>>>  Skype ID: gabbasb
>>> www.enterprisedb.co <http://www.enterprisedb.com/>m<http://www.enterprisedb.com/>
>>> *
>>> Follow us on Twitter*
>>> @EnterpriseDB
>>>
>>> Visit EnterpriseDB for tutorials, webinars, whitepapers<http://www.enterprisedb.com/resources-community>and more<http://www.enterprisedb.com/resources-community>
>>>
>>
>>
>>
>> --
>> Best Wishes,
>> Ashutosh Bapat
>> EntepriseDB Corporation
>> The Postgres Database Company
>>
>
>
>
> --
> --
> *Abbas*
>  Architect
>
> Ph: 92.334.5100153
>  Skype ID: gabbasb
> www.enterprisedb.co <http://www.enterprisedb.com/>m<http://www.enterprisedb.com/>
> *
> Follow us on Twitter*
> @EnterpriseDB
>
> Visit EnterpriseDB for tutorials, webinars, whitepapers<http://www.enterprisedb.com/resources-community>and more<http://www.enterprisedb.com/resources-community>
>



-- 
Best Wishes,
Ashutosh Bapat
EntepriseDB Corporation
The Postgres Database Company