Re: Fix some memory leaks in ecpg.addons
Am Mittwoch, dem 08.11.2023 um 12:07 -0500 schrieb Tom Lane: > "Tristan Partin" writes: > > clang and gcc both now support -fsanitize=address,undefined. These > > are > > really useful to me personally when trying to debug issues. > > Unfortunately ecpg code has a ton of memory leaks, which makes > > builds > > really painful. It would be great to fix all of them, but I don't > > have > > the patience to try to read flex/bison code. Here are two memory > > leak > > fixes in any case. > > I'm kind of failing to see the point. As you say, the ecpg > preprocessor leaks memory like there's no tomorrow. But given its > usage (process one source file and exit) I'm not sure that is worth > much effort to fix. And what does it buy to fix just two spots? Agreed, it's not exactly uncommon for tools like ecpg to not worry about memory. After all it gets freed when the program ends. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG Semantic Analysis
Hi, sorry for the double reply, but it seems my mail client's default is now to send to the list only when hitting group reply. Not good, sigh. > I have a modified version of ECPG, to which I gave the ability to > do semantic analysis of SQL statements. Where i can share it or with > whom can I discuss it? Feel free to send it to me. Thanks, Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG Semantic Analysis
Hi, > I have a modified version of ECPG, to which I gave the ability to > do semantic analysis of SQL statements. Where i can share it or with > whom can I discuss it? Feel free to send it my way. Thanks, Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: SQL/JSON functions vs. ECPG vs. STRING as a reserved word
> I looked briefly at whether we could improve that situation. > Two of the four uses of ECPGColLabelCommon in ecpg.trailer can be > converted to the more general ECPGColLabel without difficulty, > but trying to change either of the uses in var_type results in > literally thousands of shift/reduce and reduce/reduce conflicts. > This seems to be because what follows ecpgstart can be either a general > SQL statement or an ECPGVarDeclaration (beginning with var_type), > and bison isn't smart enough to disambiguate. I have a feeling that > this situation could be improved with enough elbow grease, because > plpgsql manages to solve a closely-related problem in allowing its > assignment statements to coexist with general SQL statements. But > I don't have the interest to tackle it personally, and anyway any > fix would probably be more invasive than we want to put in post-beta. Right, the reason for all this is that the part after the "exec sql" could be either language, SQL or C. It has been like this for all those years. I do not claim that the solution we have is the best, it's only the best I could come up with when I implemented it. If anyone has a better way, please be my guest. > I also wondered briefly about just changing the affected test cases, > reasoning that breaking some ECPG applications that use "string" is > less bad than breaking everybody else that uses "string". But type > "string" is apparently a standard ECPG and/or INFORMIX thing, so we > probably can't get away with that. IIRC STRING is a normal datatype for INFORMIX and thus is implemented in ECPG for its compatibility. > Hence, I propose the attached, which fixes the easily-fixable > ECPGColLabelCommon uses and adds a hard-wired special case for > STRING to handle the var_type usage. I'm fine with this approach. Thanks Tom. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> > (1) There should be no output to stderr in the tests. Why isn't > > this > > message being caught and redirected to the normal test output file? > > These are generated during the compilation of the tests with the > pre-processor, so that's outside the test runs. This is actually a deeper issue, we have no test for the compiler itself, other than the source code it generates. We do not test warnings or errors thrown by it. The topic has come up ages ago and we simply removed the test that generated the (planned) warning message. > > (2) This message is both unintelligible and grammatically > > incorrect. > > Yeah, debugging such tests would be more helpful if the name of the > DECLARE statement is included, at least. Those messages being > generated is not normal anyway, which is something coming from the > tests as a typo with the connection name of stmt_3. > > Michael, what do you think about the attached? I think what Tom was saying is that it should be either "is overwritten with" or "is rewritten to", but you raise a very good point. Adding the statement name makes the message better. I fully agree. However, it should be the other way round, the DECLARE STATEMENT changes the connection that is used. You patch removes the warning but by doing that also removes the feature that is being tested. I'm not sure what's the best way to go about it, Shall we accept to not test this particular feature and remove the warning? After all this is not the way the statement should be used, hence the warning. Or should be keep it in and redirect the warning? In that case, we would also lose other warnings that are not planned, though. Any comments? Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org signature.asc Description: This is a digitally signed message part
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> No, you're right, although I think it's implied. Maybe we need a > statement along these lines: > > > Committers are responsible for the resolution of open items that > relate > to commits they have made. Action needs to be taken in a timely > fashion, > and if there is any substantial delay in dealing with an item the > committer should provide a date by which they expect action to be > completed. The RMT will follow up where these requirements are not > being > complied with. I think that would be helpful, yes. > If they are fine by you then I accept that. After all, the reason we > want you to deal with this is not only that you made the original > commit > but because you're the expert in this area. I will commit the patch(es). Thanks. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> Sure, DECLARE does not matter as it is new. However, please note > that > the specific point I was trying to make with my link [2] from > upthread > is related to the use of cached connection names with DEALLOCATE, as > of this line in the new test declare.pgc: > EXEC SQL DEALLOCATE PREPARE stmt_2; > > And DEALLOCATE is far from being new. I'm not sure I understand. Any usage of DECLARE STATEMENT makes the file need the current version of ecpg anyway. On the other hand DEALLOCATE did not change its behavior if no DECLARE STATEMENT was issued, or what did I miss? Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org signature.asc Description: This is a digitally signed message part
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> Agreed. How is this, which already exists? > > https://wiki.postgresql.org/wiki/Release_Management_Team That I know, but I don't think it covers the issues we, or I, had up- thread. Or do I miss something? Speaking of RMT, Andrew, Michael, Peter, will you make the final decision whether we commit Kuroda-san's patches? They are fine by me. Another pair of eyes would be nice, though. maybe you could have another look, Horiguchi-san? Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> I think my point was that committers should be required to understand > the RMT process, and if we need to communicate that better, let's do > that. I don't think it should be the responsibility of RMT members > to > communicate the RMT process every time they communicate with someone, > unless someone asks. Completely agreed, my point was that documenting the process to some extend would be helpful. For obvious reasons I'm the wrong person to do that, though. :) Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> I do think all committers need to understand the role of the RMT so > they > can respond appropriately. Do we need to communicate this better? Being the one who assumed a different procedure, yes please. :) Thanks, Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> Okay. So you mean to change DESCRIBE and DEALLOCATE to be able to > handle cached connection names, as of [1]? For [DE]ALLOCATE, I agree Yes, at least technically. I think DESCRIBE should accept the cached connection name, although it won't really matter. > that it could be a good thing. declare.pgc seems to rely on that > already but the tests are incorrect as I mentioned in [2]. For > DESCRIBE, that provides data about a result set, I find the > assignment > of a connection a bit strange, and even if this would allow the use > of > the same statement name for multiple connections, it seems to me that > there is a risk of breaking existing applications. There should not > be that many, so perhaps that's fine anyway. I don't think we'd break anything given that DECLARE STATEMENT is new. Also please keep in mind that you can use EXEC SQL AT ... DESCRIBE ...; already anyway. Again, not very meaningful but why should we accept a connection one way but not the other? Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org signature.asc Description: This is a digitally signed message part
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Peter, > I think that there was a snowballing effect here. We both made > mistakes that compounded. I apologize for my role in this whole mess. Completely agreed. I think we both took things for granted that the other one didn't take into account at all. I'm sorry about that, too. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> > Again agreed, please keep in mind, though, that I didn't notice I > > was > > being chased until Peter's first email. > > I was asked by the RMT to contact one of your employees, and I did, > to > tell you that the RMT was looking for feedback from you on an ecpg > issue. Not sure if that was before or after Peter's email. I think that was before, at that point I still thought it was nothing time sensitive. And unfortunately it didn't register that RMT was involved at all. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Peter, > I think that this must be a cultural thing. I can see how somebody > would see the third person style as overly formal or stilted. An > interpretation like that does make sense to me. But I knew of no > reason why you might find that style made the message offensive. It > was never intended to denigrate. This explains why it felt so difficult to make myself clear. I was feeling exactly the same, just the other way round. > I don't know you all that well, but we have talked for quite a while > on a few occasions. I got the sense that you are a decent person from > these conversations. I have no possible reason to denigrate or insult > you. In general I try to be respectful, and if I ever fail it's not > because I didn't care at all. Anybody that knows me well knows that I > am not a mean spirited person. I never though that. Maybe we should have quickly talked things out. Email tends to be a bad medium for communication, especially when it goes wrong. :) > If this is just an unfortunate misunderstanding, as I suspect it is, > then I would be happy to let it go, and treat it as something to > learn > from. Agreed, me too. I'd like to apologize for not answering your email the way I should have. Honestly it never occurred to me. Maybe that's because I'm used to other procedures, or because I never had to converse with the RMT, or maybe, quite simple, because I lacked the time to think it through, the original issue that kind of started this whole mess. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> > > I don't want to upset anybody for any reason. I regret that my > > > words > > > have upset you, but I think that they were misinterpreted in a > > > way > > > that I couldn't possibly have predicted. The particular aspect of > > > > I strongly object to that. It's pretty obvious to me that > > addressing > > people in third person is very offending. > > So, you object to him referring to you in the third person in an > email, > and you object to him saying it was "misinterpreted". Are you going > to > object to my email too? No, of course not. And sorry for not being precise enough, I only objected to the prediction part, but I agree, I take the objection back. I guess it's as difficult for Peter to understand why this is offensive as it is for me to not see it as such. > I think it might have been in the third person because at that point, > Peter didn't expect a reply from you, and put you on the "TO" line > merely as a courtesy. He could have put out an email about reverting > the patch without you on the email header at all, I guess --- then he > could have referred to you without offending you. Right, that was my only problem originally. It seemed difficult to bring that point over. > Let me be practical here --- the more someone has to be chased for a > reply, the less confidence they have in that person. If the RMT > contacts you about something, and obviously they have had to take > usual > efforts to contact you, the more it is on you to give a full report > and > a timeline of when you will address the issue. If they had to chase > you > around, and you gave them a short answer, the less confidence they > have > in this getting resolved in a timely manner. Again agreed, please keep in mind, though, that I didn't notice I was being chased until Peter's first email. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Hi, > FWIW, I don't think that the phrasing of Peter's email is > disrespectful. As I read it, it simply states that the RMT has made a As I said before, it might be a cultural difference. What I don't understand is, that a simple "Hi Michael, this is what the RMT decided:" would have been sufficient to make this email okay. I take offense in being addressed in third person only. > strongly that being a member of the RMT is a pretty thankless task, That I agree with. > On the substance of the issue, one question that I have reading over > this thread is whether there's really a bug here at all. It is being > represented (and I have not checked whether this is accurate) that > the > patch affects the behavior of DECLARE, PREPARE, and EXECUTE, but not > DESCRIBE, DEALLOCATE, DECLARE CURSOR .. FOR, or CREATE TABLE AS > EXECUTE. However, it also seems that it's not entirely clear what the > behavior ought to be in those cases, except perhaps for DESCRIBE. If > that's the case, maybe this doesn't really need to be an open item, > and maybe we don't therefore need to talk about whether it should be > reverted. Maybe it's just a feature that supports certain things now > and in the future, after due reflection, perhaps it will support > more. The way I see it we should commit the patch that makes more statements honor the new DECLARE STATEMENT feature. I don't think we can change anything for the worse by doing that. And other databases are not really strict about this either. > I would be interested in hearing your view, and that of others, on > whether this is really a bug at all. I think the question is more which version of the patch we commit as it does increase the functionality of ecpg. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> question with a question mark. Despite the fact that it is generally > understood that "committers own their own items", and that the RMT > exists as a final check on that. This does not contradict my opinion, but anyway. > Clearly we disagree about this. I don't think that there is anything > to be gained from discussing this any further, though. I suggest that > we leave it at that. Agreed. > I don't want to upset anybody for any reason. I regret that my words > have upset you, but I think that they were misinterpreted in a way > that I couldn't possibly have predicted. The particular aspect of I strongly object to that. It's pretty obvious to me that addressing people in third person is very offending. > last > Friday's email that you took exception to was actually intended to > convey that it was not personal. Remember, my whole ethos is to avoid > strong RMT intervention when possible, to make it impersonal. My > framing of things had the opposite effect to the one I'd intended, > ironically. Let me stress again that the third person part is the bad thing in my opinion, not the rest of the words. > How could anybody on the RMT judge what was going on sensibly? There > was *zero* information from you (the original committer, our point of > contact) about an item that is in a totally unfamiliar part of the > code to every other committer. We were effectively forced to make > very > conservative assumptions about the deadline. I think that it's very > likely that this could have been avoided if only you'd engaged to > some > degree -- if you had said it was a short deadline then we'd likely > have taken your word for it, as the relevant subject matter expert > and > committer in charge of the item. But we were never given that choice. The same holds the other way round, I only understood later that you wanted more information. Had I known that earlier, I would have gladly given them. > > Well, you did lay out what the decision would be and I fully agreed > > with it. So again, what was there to do? Had you asked me if I > > agreed, > > I would told you. > > If the patch being reverted was so inconsequential to you that you > didn't even feel the need to write a brief email about it, why did > you > commit it in the first place? I just don't understand this at all. I'm getting very tired of you accusing me of things I neither said nor did. Please stop doing that or show me the email where I said the patch was "inconsequential"? As for writing a brief email, please read all the other emails in this thread, I've explained myself repeatedly already. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> My email of July 30 was itself pretty strongly worded, but went > unanswered for a full week. Not even a token response. If that > doesn't > count as "ignoring the RMT", then what does? How much time has to > pass > before radio silence begins to count as "ignoring the RMT", in your > view of things? A month? More? If you want me to answer, how about asking a question? Or telling me that you'd like some feedback? I don't see how I should know that you expect a reply to a simple statement of facts. > Okay, I understand that now. And? Do you care at all? > As Andrew pointed out, there is a general expectation that committers > take care of their own open items. That doesn't mean that they are > obligated to personally fix bugs. Just that the final responsibility > to make sure that the issue is resolved rests with the committer. > This > is one of the few hard rules that we have. Sure, I don't question that. Again, I knew about the issue, only misjudged it in the beginning. Your email from July 30 did show me that it was more urgent but still didn't create the impression that there was such a short deadline. In my opinion my prior email already explained that I was on it, but couldn't give an estimate. > As I've said before, RMT-driven revert is something that I see as an > option of last resort. The RMT charter doesn't go quite that far, but > I'd argue that my interpretation is quite natural given how committer > responsibility works in general. In other words, I personally believe > that our bottom-up approach is on balance a good one, and should be > preserved. Fair enough, to me a revert is a revert, no matter who issues it. > Maybe the issue is muddied by the fact that we each have different > views of the community process, at a high level (I'm unsure). Unlike > you, I don't believe that RMT-driven revert is "a reasonable > procedure". I myself see the RMT's power to resolve open items in > this > way as a necessary evil. Something to be avoided at all costs. Why > should people that don't know anything about ecpg make decisions > about > ecpg? In general they should not. Well, you did lay out what the decision would be and I fully agreed with it. So again, what was there to do? Had you asked me if I agreed, I would told you. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Dear Kuroda-san, > I perfectly missed mails and 8/9 was a national holiday. > I must apologize about anything. Very sorry for inconvenience. No need to, non of it is your fault. > I summarize the thread. Thank you so much, this is very, very helpful. > As you might know DECLARE STATEMENT has been added from PG14, but I > found that connection-control feature cannot be used for DEALLOCATE > and DESCRIBE statement (More details, please see patches or ask me). > But we have a question - what statement should use the associated > connection? Obviously DEALLOCATE statement should follow the linked > connection because the statement uses only one sql identifier. In > DESCRIBE or any other descriptor-related statements, however, I think > it is non-obvious because they have also descriptor-name. Currently I > made patches that includes about DESCRIBE, but I'm wondering this > approach is correct. I want you to ask your opinion about the scope > of > DECLARE STATEMENT. I've been reading through quite a few documents to come up with a good reason one way or the other, but as you already pointed out yourself, other database systems seem to not be consequent about the usage either. Unfortunately I didn't find my copy of the SQL standard. But then I kind of doubt it has much to say about this particular issue. Anyway, I'm currently leaning towards including DESCRIBE in the list of statements that are influenced by DECLARE STATEMENT. My reason is that DESCRIBE can be issued with an AT clause already, regardless whether it makes sense or not. Or in other words, if we allow it to get a connection name one way, why should we forbid the other way. To me this seems to be more confusing than the current situation. The alternative would be to forbid using an AT clause with DESCRIBE, which would definitely be a compatibility change, although, again, not a functional one. > Coding is not hard hence I think we can fix this until the end of > Sep. > if we set a policy correctly and have reviewers. Fully agreed. That's why a short glance at the patch didn't suffice to answer this. I didn't see any issues with the patch so far. Please send it to me once its finished (or is it already?) and I'll give it a run, too. Hopefully I caught up on all emails and didn't miss parts of the thread. Thanks, Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> I think that it's crystal clear what I meant in the email of July 30. > I meant: it's not okay that you're simply ignoring the RMT. You > hadn't > even made a token effort at that point. For example you didn't give > the proposed fix a cursory 15 minute review, just so we had some very > rough idea of where things stand. You still haven't. How do you know I didn't spend 15 minutes looking at the patch and the whole email thread? I surely did and it was more than 15 minutes, but not enough to give a meaningful comment. If you can do it in 15 minutes, great for you, I cannot. The meaning of your email of July 30 was crystal clear, yes. It means you'd revert the patch if I didn't resolve the issue. This is literally what it says. If you meant to say "It's not okay that you're simply ignoring the RMT. You hadn't even made a token effort at that point." it might have been helpful if you said that, instead of having me guess if there was a hidden meaning in your email. Besides, I have not ignored the RMT. I don't know why you keep insisting on something that is simply not true. > My understanding of what you're taking issue with (perhaps a flawed > understanding) is that you think that you have been treated unfairly > or arbitrarily in general. That's why I brought up the email of July > 30 yesterday. So my point was: no, you haven't been treated unfairly. Yes, this is a flawed understanding. I'm sorry you came to that understanding, I though my emails were pretty clear as to what I was objecting to. > If you only take issue with the specific tone and tenor of my email > from Friday (the email that specified a deadline), and not the > content > itself, then maybe the timeline and the wider context are not so > important. > > I am still unsure about whether your concern is limited to the tone > of > the email from Friday, or if you also take exception to the content > of > that email (and the wider context). At the risk of repeating myself, my concern is *only* the rude and disrespectful way of addressing me in the third person while talking to me directly. Again, I though I made that clear in my first email about the topic and every one that followed. > Perhaps the tone of my email from Friday was unhelpful. Even still, I > am surprised that you seem to think that it was totally outrageous -- > especially given the context. It was the first email that you The context never makes a derogative communication okay, at least not in my opinion. > responded to *at all* on this thread, with the exception of your > original terse response. I am not well practised in communicating > with > a committer that just doesn't engage with the RMT at all. This is all > new to me. I admit that I found it awkward to write the email for my > own reasons. I was *never* asked for *any* response in *any* email, save the original technical discussion, which I did answer with telling people that I'm lacking time but will eventually get to it. Just to be precise, your email from July 30 told me and everyone how this will be handled. A reasonable procedure, albeit not one we'd like to see happen. But why should I answer and what? It's not that you bring this up as a discussion point, but as a fact. > I brought up flexibility to point out that this could have been > avoided. If you needed more time, why didn't you simply ask for it? The first conversation that brought up the time issue was your email that started this thread. There you set a deadline which I understand and accept. But then I never said a word against it, so the question remains, why accusing me of something I never did? > Again, the scope of what you're complaining about was (and still is) > unclear to me. I'm sorry, but I have no idea how to explain it more clearly. I'm not asking for any favor or special treatment, I just ask to be treated like a person. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
On Sat, 2021-08-07 at 15:31 -0700, Peter Geoghegan wrote: > On Sat, Aug 7, 2021 at 12:43 PM Michael Meskes > wrote: > > Again, I didn't know the RMT was expecting anything from me. Yes, I > > knew I needed to spend some time on a technical issues, but that's > > exactly the information I had at the time. > > As Andrew mentioned, I sent you an email on the 30th -- a full week > prior to the email that formally timeboxed this open item. That > earlier email is here: > > https://postgr.es/m/CAH2-Wzk=qxtsp0h5ekv92eh0u22hvmqlhgwyp4_fa3ytiei...@mail.gmail.com This email said nothing about sending a status update or a deadline or any question at all, only that you'd revert the patch if I was unable to resolve the issue. So what's your point? > I also talked about the RMT in the third person. My intent was to > make So? It's okay to disrespect a person if you mention the team that you are representing in the third person, too? > the message legalistic and impersonal. That's what is driving our > thinking on this -- the charter of the RMT. Please show me where the charter says that disrespecting a person is fine. And while we're add it, does the code of conduct say anything about the way people should be treated? > We're all volunteers, just like you. I happen to be a big believer in > our culture of personal ownership and personal responsibility. But > you > simply haven't engaged with us at all. Which I tried to explain several times, but apparently not well enough. Let me give you a short rundown from my perspective: - A patch is sent that I mistakenly thought was a new feature and thus did not apply time too immediately. - After a while I get an email from you as spokesperson of the RMT that if this is not fixed it'll have to be reverted eventually. - I learn that Andrew tried to reach me. Again, I believe you Andrew, that you sent the email, but I never saw it. Whether it's some filtering or a user error that made it disappear, I have no idea, but I'm surely sorry about that. - I receive that email we keep talking about, the one in which you treat me like I'm not even worth being addressed. > You didn't say anything at all, which speaks for itself. And makes it > impossible for us to be flexible. Which flexibility did I ask for? It'd be nice if you only accused me of things I did. Honestly I do not understand you at all. You keep treating me like I was asking for anything unreasonable while I'm only trying to explain why I didn't act earlier. The only issue I have is the rude treatment you gave me. Just for the record, of course I'm going to look into the issue before your deadline and will send a status update. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
> That's simply not true. Andrew Dunstan reached out personally and got > no response. He then reached out through a backchannel (a direct > coworker of yours), before finally getting a single terse response > from you here. You do know that I did not receive any email from Andrew. After all I explained this to the backchannel you mentioned. I do not know what happened, I do not even know if it was one email or several, but I checked everything, there simply is no such email in my mailbox. > Every one of us has a life outside of PostgreSQL. An individual > contributor may not be available, even for weeks at a time. It > happens. The RMT might well have been much more flexible if you > engaged with us privately. There has not been a single iota of > information for us to go on. That's why this happened. Again, I didn't know the RMT was expecting anything from me. Yes, I knew I needed to spend some time on a technical issues, but that's exactly the information I had at the time. > > The tone was formal and impersonal because it represented the > position > of the RMT as a whole (not me personally), and because it's a > particularly serious matter for the RMT. It concerned the RMT > exercising its authority to resolve open items directly, in this case > by calling for a revert. This is the option of last resort for us, > and > it was important to clearly signal that we had reached that point. Please read my prior email completely, I did go into detail about what I meant with tone. I don't mind a formal wording and I completely agree that a decision has to be made at some point. I was wrong in thinking there was more time left, but that's also not the point. The point is that you talk *about* me in the third person in an email you address at me. It might be normal for you, but in my neck of the woods this is very rude behavior. > No other committer (certainly nobody on the RMT) knows anything about > ecpg. How much longer were you expecting us to wait for a simple > status update? Where did I say I expect you to wait? How could I even do that given that I didn't even know you were waiting for a status update from me? Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Hi Peter,
> The RMT continues to be concerned about the lack of progress on this
> open item, especially the lack of communication from Michael Meskes,
> the committer of the original patch (commit ad8305a). We are prepared
> to exercise our authority to resolve open items directly. The only
> fallback option available to us is a straight revert of commit
> ad8305a.
>
> We ask that Michael Meskes give a status update here no later than
> 23:59 on Fri 13 Aug ("anywhere on earth" timezone). This update
> should
> include a general assessment of the problem, a proposed resolution
> (e.g., committing the proposed patch from Hayato Kuroda), and an
> estimate of when we can expect the problem to be fully resolved. If
> Michael is unable to provide a status update by that deadline, or if
> Michael is unable to commit to a reasonably prompt resolution for
> this
> open item, then the RMT will call for a revert without further delay.
>
> The RMT's first responsibility is to ensure that PostgreSQL 14 is
> released on schedule. We would prefer to avoid a revert, but we
> cannot
> allow this to drag on indefinitely.
I get it that the goal is to release PostgreSQL 14 and I also get it
that nobody is interested in the reasons for my slow reaction. I even,
at least to an extend, understand why nobody tried reaching out to me
directly. However, what I cannot understand at all is the tone of this
email. Is this the new way of communication in the PostgreSQL project?
Just to be more precise, I find it highly offensive that you address an
email only to me (everyone else was on CC) and yet do not even try to
talk to me, but instead talk about me as a third person. I find this
very disrespectful.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
All, > between DECLARE and the past queries qualify as an open item. I am > adding Michael Meskes in CC. I got to wonder how much of a > compatibility break it would be for DEALLOCATE and DESCRIBE to handle > EXEC SQL AT in a way more consistent than DECLARE, even if these are > bounded to a result set, and not a connection. I just wanted to let you know that I'm well aware of this thread but need to get through my backlog before I can comment. Sorry for the delay. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org signature.asc Description: This is a digitally signed message part
Re: [PATCH] ecpg: fix progname memory leak
On Thu, 2020-10-08 at 12:27 -0400, Bruce Momjian wrote: > On Thu, Oct 8, 2020 at 10:35:41AM +0900, Michael Paquier wrote: > > On Wed, Oct 07, 2020 at 02:31:54PM +0300, Maksim Kita wrote: > > > Fix progname memory leak in ecpg client. > > > Issue taken from todo list https://wiki.postgresql.org/wiki/Todo. > > > > FWIW, I don't see much point in doing that. For one, we have a > > more-or-less established rule that progname remains set until the > > application leaves, and there are much more places where we leak > > memory like that. As one example, just see the various places > > where > > we use pg_strdup for option parsing. At the end, it does not > > really > > matter as these are applications running for a short amount of > > time. > > Agreed, but what does the TODO item mean then? > > Fix small memory leaks in ecpg > Memory leaks in a short running application like ecpg are > not really > a problem, but make debugging more complicated > > Should it be removed? I'd say yes, let's remove it. Actually I wasn't even aware it's on there. While I agree that it makes debugging of memory handling in ecpg more difficult, I don't see much of a point in it. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: ECPG: proposal for new DECLARE STATEMENT
> This patch has now been silent for quite a while, unless someone is > interested > enough to bring it forward it seems about time to close it. I am interested but still short on time. I will definitely look into it as soon as I find some spare minutes. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Re: MinGW compiler warnings in ecpg tests
> These files don't use our printf replacement or the c.h porting > layer, > so unless we want to start doing that, I propose the attached patch > to > determine the appropriate format conversion the hard way. I don't think such porting efforts are worth it for a single test case, or in other words, if you ask me go ahead with your patch. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: A problem presentaion about ECPG, DECLARE STATEMENT
> > > Is it acceptable for PG12? > > In general absolutely. > > It seems far too late to be considering any major rewrite for v12; > even assuming that there was consensus on the rewrite being an > improvement, which I bet there won't be. Oops, I read 13. Yes, it's obviously way too late for 12. Sorry for the noise. In this case I'd like to details about what is wrong with the implementation. Thanks. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: A problem presentaion about ECPG, DECLARE STATEMENT
Hi Kuroda-san, > This feature, committed last February, has some bugs. > ... > * This syntax does not have oracle compatibility. This in itself is not a bug. If the syntax is not standard compliant, then it's a bug. That of course does not mean we would not like to be Oracle compatible where possible. > In order to modify bugs, I think many designs, implementations, > and specifications should be changed. I hope the authors of said patch speak up and explain why they implemented it as is. > Is it acceptable for PG12? In general absolutely. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [PATCH] memory leak in ecpglib
Hi, > Memory leaks occur when the ecpg_update_declare_statement() is called > the second time. > ... I'm going to commit this patch HEAD, this way we can see if it works as advertised. It does not hurt if it gets removed by a rewrite. Thanks for finding the issue, Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: Bug: ECPG: Cannot use CREATE AS EXECUTE statemnt
Matsumura-san, > I noticed that CREATE AS EXECUTE with using clause needs a new > implementation that all parameters in using clause must be embedded > into > expr-list of EXECUTE in text-format as the following because there is > no interface of protocol for our purpose. > It spends more time for implementing. Do you have any advice? > ... Unfortunately no, I have no advice. Originally all statements needed this treatment. :) Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: coverage additions
On Thu, 2019-05-30 at 17:54 -0400, Tom Lane wrote: > Alvaro Herrera writes: > > Apparently, for ecpg you have to do "make checktcp" in order for > > some of > > the tests to run, and "make check-world" doesn't do that. Not sure > > what's a good fix for this; do we want to add "make -C > > src/interfaces/ecpg/test checktcp" to what "make check-world" does, > > or do we rather what to add checktcp as a dependency of "make > > check" in > > src/interfaces/ecpg? > > Or do we just not want this test to be run by default, and thus I > > should > > add "make -C src/interfaces/ecpg/test checktcp" to > > coverage.pg.org's > > shell script? > > I believe it's intentionally not run by default because it opens up > an externally-accessible server port. Correct, iirc. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
> This patch seems to have little incidence on the stability, but FWIW > I > am not cool with the concept of asking for objections and commit a > patch only 4 hours after-the-fact, particularly after feature freeze. This was only done to beat the pg_indent run as Tom pointed out. I figured worse case we can revert the patch if people object. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL signature.asc Description: This is a digitally signed message part
Re: SQL statement PREPARE does not work in ECPG
> None here. You might want to get it in in the next 12 hours or so > so you don't have to rebase over a pgindent run. Thanks for the heads-up Tom, pushed. And thanks to Matsumura-san for the patch. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
Hi Matsumura-san, > (1) > I attach a new patch. Please review it. > ... This looks good to me. It passes all my tests, too. There were two minor issues, the regression test did not run and gcc complained about the indentation in ECPGprepare(). Both I easily fixed. Unless somebody objects I will commit it as soon as I have time at hand. Given that this patch also and mostly fixes some completely broken old logic I'm tempted to do so despite us being pretty far in the release cycle. Any objections? > (2) > I found some bugs (two types). I didn't fix them and only avoid bison > error. > > Type1. Bugs or intentional unsupported features. > - EXPLAIN EXECUTE > - CREATE TABLE AS with using clause > ... Please send a patch. I'm on vacation and won't be able to spend time on this for the next couple of weeks. > Type2. In multi-bytes encoding environment, a part of character of > message is cut off. > > It may be very difficult to fix. I pretend I didn't see it for a > while. > ... Hmm, any suggestion? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
Hi Matsumura-san, > I defined the following specifications. Please review it. > > * ECPG can accept any valid PREPARE AS statement. > * A char-type host variable can be used as the statement name of > PREPARE AS, > but its value is constrained by the specification of PREPARE AS. > (e.g. the name cannot include double quotation.) > * The above also allows the following. It's a bit strange but there > is no reason > for forbidding. > prepare :st(type_list) as select $1 > * ECPG can accept EXECUTE statement with expression list that is > allocated > by both PREPARE FROM and PREPARE AS under the following > constraints: > - It must not include a using-clause. > - The statement name must follow to the specification of PREPARE > AS. This look very reasonable to me. I'm completely fine with this restriction being placed on PREPARE FROM. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: fix memory overflow in ecpg preproc module
Hi, > I have found a potential memory overflow in ecpg preproc module. > ... Thanks for finding and fixing, committed. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
Hi Matsumura-san, > If we also allow the statement name including white space in PREPRARE > AS, > we have to make backend parser to scan it as IDENT. Correct, without quoting would only work when using PQprepare() et al. > I cannot think of anything. > I may notice if I try to merge. Thanks. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
Matsumura-san, > Therefore, I think that the quoting statement name is needed in > PREPARE/EXECUTE case, too. I agree that we have to accept a quoted statement name and your observations are correct of course, I am merely wondering if we need the escaped quotes in the call to the ecpg functions or the libpq functions. > > I would prefer to merge as much as possible, as I am afraid that if > > we > > do not merge the approaches, we will run into issues later. There > > was a > > reason why we added PQprepare, but I do not remember it from the > > top of > > my head. Need to check when I'm online again. > > I will also consider it. Thank you. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
Hi all and thank you Matsumura-san. > Excuse: > It doesn't include regression tests and pass them. > Because I must reset all expected C program of regression. > # I add an argument to ECPGdo(). Sure, let's do this at the very end. > 1. Specification > It accepts the following .pgc. > I confirmed it works well for AT clause. > All results for st1 and st2 are same. I have a similar text case and can confirm that the output is the same for both ways of preparing the statement. > 2. Behavior of ecpglib > (1) PREPARE with AS clause > Ecpglib sends the PREPARE statement to backend as is. (using > PQexec). > > (2) EXECUTE with parameter list > Ecpglib sends the EXECUTE statement as is (using PQexec), but all > host variables in > the list are converted to string-formatted and embedded into the > EXECUTE statement. > > (3) PREPARE with FROM clause (not changed) > Ecpglib sends 'P' libpq-message with statement (using PQprepare). > > (4) EXECUTE without parameter list (not changed) > Ecpglib sends 'B' libpq-message with parameters. (using > PQexecPrepared). > > > 3. Change of preprocessor > > - I add ECPGst_prepare and ECPGst_execnormal. >ECPGst_prepare is only for (1) and ECPGst_execnormal is only for > (2). ># I think the names are not good. > > - I add one argument to ECPGdo().p It's for prepared statement name. One question though, why is the statement name always quoted? Do we really need that? Seems to be more of a hassle than and advantage. > 4. > I wonder whether I should merge (3) to (1) and (4) to (4) or not. I would prefer to merge as much as possible, as I am afraid that if we do not merge the approaches, we will run into issues later. There was a reason why we added PQprepare, but I do not remember it from the top of my head. Need to check when I'm online again. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: ECPG regression with DECLARE STATEMENT support
> I attached a simple bug-fixing patch. I'm not happy with the situation, but don't see a better solution either. Therefore I committed the change to get rid of the regression. Thanks. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: ECPG regression with DECLARE STATEMENT support
Hi, > Commit bd7c95f0c1a38becffceb3ea7234d57167f6d4bf add DECLARE > STATEMENT support to ECPG. This introduced the new rule > for EXEC SQL CLOSE cur and with that it gets transformed into > ECPGclose(). > > Now prior to the above commit, someone can declare the > cursor in the SQL statement and "CLOSE cur_name" can be > also, execute as a normal statement. That still works, the difference in your test case is that the DECLARE statement is prepared. > Example: > > EXEC SQL PREPARE cur_query FROM "DECLARE cur1 CURSOR WITH HOLD FOR > SELECT count(*) FROM pg_class"; > EXEC SQL PREPARE fetch_stmt FROM "FETCH next FROM cur1"; > EXEC SQL EXECUTE cur_query; > EXEC SQL EXECUTE fetch_stmt INTO :c; > EXEC SQL CLOSE cur1; > > With commit bd7c95f0c1, "EXEC SQL CLOSE cur1" will fail > and throw an error "sqlcode -245 The cursor is invalid". > > I think the problem here is ECPGclose(), tries to find the > cursor into "connection->cursor_stmts" and if it doesn't > find it there, just throws an error. Maybe require fix > into ECPGclose() - rather than throwing an error continue > executing statement "CLOSE cur_name" with ecpg_do(). The problem as I see it is that the cursor is known to the backend but not the library. Takaheshi-san, Hayato-san, any idea how to improve the situation to not error out on statements that used to work? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
Hi all, > ... > But it doesn't allow to use host variable in parameter clause of > EXECUTE statement like the following. > I'm afraid that it's not usefull. I will research the standard and > other RDBMS. > If you have some information, please adivise to me. This also seems to be conflicting with bd7c95f0c1a38becffceb3ea7234d57167f6d4bf. If we want to keep this commit in for the release, I think we need to get these things fixed. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
Hi Matsumura-san,
> I must use a midrule action like the following that works as
> expected.
> I wonder how to write the replacement to ecpg.addons.
> I think it's impossible, right? Please give me some advice.
You are right, for this change you have to replace the whole rule. This
cannot be done with an addon. Please see the attached for a way to do
this, untested though.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer
index 1d58c778d9..be6c9d0ae9 100644
--- a/src/interfaces/ecpg/preproc/ecpg.trailer
+++ b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -1,5 +1,26 @@
/* src/interfaces/ecpg/preproc/ecpg.trailer */
+PrepareStmt: PREPARE prepared_name prep_type_clause AS
+{
+prepared_name = $2;
+prepare_type_clause = $3;
+is_in_preparable_stmt = true;
+}
+PreparableStmt
+{
+$$.name = prepared_name;
+$$.type = prepare_type_clause;
+$$.stmt = $6;
+is_in_preparable_stmt = false;
+}
+| PREPARE prepared_name FROM execstring
+ {
+$$.name = $2;
+$$.type = NULL;
+$$.stmt = $4;
+}
+ ;
+
statements: /*EMPTY*/
| statements statement
;
diff --git a/src/interfaces/ecpg/preproc/ecpg.type b/src/interfaces/ecpg/preproc/ecpg.type
index 519b737dde..1c68095274 100644
--- a/src/interfaces/ecpg/preproc/ecpg.type
+++ b/src/interfaces/ecpg/preproc/ecpg.type
@@ -145,3 +145,5 @@
%type var_type
%type action
+
+%type PrepareStmt
diff --git a/src/interfaces/ecpg/preproc/parse.pl b/src/interfaces/ecpg/preproc/parse.pl
index 6f67a5e942..cafca22f7a 100644
--- a/src/interfaces/ecpg/preproc/parse.pl
+++ b/src/interfaces/ecpg/preproc/parse.pl
@@ -64,6 +64,7 @@ my %replace_types = (
'stmtblock' => 'ignore',
'stmtmulti' => 'ignore',
'CreateAsStmt' => 'ignore',
+ 'PrepareStmt' => 'ignore',
'DeallocateStmt' => 'ignore',
'ColId' => 'ignore',
'type_function_name' => 'ignore',
Re: SQL statement PREPARE does not work in ECPG
Hi Matsumura-san, > I will read README.parser, ecpg.addons, and *.pl to understand. Feel free to ask, when anything comes up. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
Hi Matsumura-san,
> But I will probably be late because I don't understand parse.pl very
> well.
> I think that the following rule is made by parse.pl.
>
>PreparableStmt:
>SelectStmt
>{
>is_in_preparable_stmt = true; <--- I want to add it.
>$$ = $1;
> }
> | InsertStmt
> .
The only way to add this is by creating a replacement production for
this rule. parse.pl cannot do it itself.
> I will use @1 instend of $$1 because the replacing is almost same as
> the existing replacing function in ecpglib.
> Is it good?
I'd say so.
Thanks.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [Bug Fix] ECPG: could not use set xxx to default statement
> Not seeing any motion on this, here's a draft patch to make these > scripts complain about missing semicolons. Against the current > gram.y (which contains 2 such errors, as Michael noted) you > get output like Thanks Tom for looking into this. Are we agreed then that we want gram.y to have semicolons? > '/usr/bin/perl' ./parse.pl . < ../../../backend/parser/gram.y > > preproc.y > unterminated rule at ./parse.pl line 370, <> line 1469. > make: *** [preproc.y] Error 255 > make: *** Deleting file `preproc.y' > > That's not *super* friendly, but it does give you the right line > number > to look at in gram.y. We could adjust the script (and the Makefile) > further so that the message would cite the gram.y filename, but I'm > not > sure if it's worth the trouble. Thoughts? IMO it's not worth it. We all know where the grammar is and that the ecpg tools only parse that one file. Why putting effort into writing it down too? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
Takahashi-san, > It works well for my statement > > "EXEC SQL PREPARE test_prep (int) AS SELECT id from test_table where > id = $1;". > > However, since data type information is not used, it does not works > well > for prepare statements which need data type information such as > "EXEC SQL PREPARE test_prep (int, int) AS SELECT $1 + $2;". Yes, that was to be expected. This is what Matsumura-san was trying to fix. However, I'm not sure yet which of his ideas is the best. > It also works. > (Actually, I wrote "EXEC SQL EXECUTE test_prep (:i) INTO :ID;".) Ok, thanks. That means the parser has to handle the list of execute arguments differently, which in hindsight is pretty obvious. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
Takahashi-san, > EXEC SQL EXECUTE test_prep (2); Could you please also verify for me if this works correctly if you use a variable instead of the const? As in: EXEC SQL BEGIN DECLARE SECTION; int i=2; EXEC SQL END DECLARE SECTION; ... EXEC SQL EXECUTE test_prep (:i); I guess the problem is that constants in this subtree are move to the output literally instead treated as parameters. Thanks. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
Takahashi-san,
> I tried as follows.
> ...
> Unfortunately, this does not work.
> ECPGst_execute seems good, but prepare statement is the same as my
> first post.
Ah right, my bad. The workaround should have been:
EXEC SQL PREPARE test_prep from "SELECT id from test_table where id =
$1";
EXEC SQL EXECUTE test_prep using 2;
> It fails with "PostgreSQL error : -202[too few arguments on line
> 16]".
>
> This error is caused by following source code.
> ...
> I think next_insert() should ignore "$n" in the case of SQL statement
> PREPARE.
Actually I am not so sure.
> In addition, we should fix following, right?
>
> (1)
> As Matsumura-san wrote, ECPG should not produce '"' for SQL statement
> PREPARE.
Why's that?
> (2)
> ECPG should produce argument for execute statement such as "EXEC SQL
> EXECUTE test_prep (2);"
Correct.
As for the PREPARE statement itself, could you try the attached small
patch please.
This seems to create the correct ECPGPrepare call, but I have not yet
tested it for many use cases.
Thanks.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
diff --git a/src/interfaces/ecpg/preproc/ecpg.addons b/src/interfaces/ecpg/preproc/ecpg.addons
index e8052819b0..bfd76960a2 100644
--- a/src/interfaces/ecpg/preproc/ecpg.addons
+++ b/src/interfaces/ecpg/preproc/ecpg.addons
@@ -22,12 +22,7 @@ ECPG: stmtUpdateStmt block
ECPG: stmtExecuteStmt block
{ output_statement($1, 1, ECPGst_execute); }
ECPG: stmtPrepareStmt block
- {
- if ($1.type == NULL || strlen($1.type) == 0)
- output_prepare_statement($1.name, $1.stmt);
- else
- output_statement(cat_str(5, mm_strdup("prepare"), $1.name, $1.type, mm_strdup("as"), $1.stmt), 0, ECPGst_normal);
- }
+ { output_prepare_statement($1.name, $1.stmt); }
ECPG: stmtTransactionStmt block
{
fprintf(base_yyout, "{ ECPGtrans(__LINE__, %s, \"%s\");", connection ? connection : "NULL", $1);
Re: SQL statement PREPARE does not work in ECPG
Matsumura-san, > Maybe, there is no work-around. Did you analyze the bug? Do you know where it comes from? > For supporting it, there are two steps. Could you please start with explaining where you see the problem? I'm actually not sure what you are trying to do here. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: SQL statement PREPARE does not work in ECPG
> I think SQL statement PREPARE *without* parameter is supported, > but one with parameter is not supported (or has some fatal bugs). It surely should be supported. >> I wrote the source code as follows. >> >> >> EXEC SQL PREPARE test_prep (int) AS SELECT id from test_table where id = $1; >> EXEC SQL EXECUTE test_prep (2); >> Please try this instead: EXEC SQL PREPARE test_prep (int) AS SELECT id from test_table where id = $1; EXEC SQL EXECUTE test_prep using 2; This should work. And yes, it does look like a bug to me, or better like changes in the backend that were not synced to ecpg. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [Bug Fix] ECPG: could not use set xxx to default statement
Higuchi-san, > I attached the patch which cope with missing semicolons. > Previous parse.pl find semicolon and dump data to buffer. When > attached patch's parse.pl find new tokens before finding a semicolon, > it also dumps data to buffer. It just occurred to me that check_rules.pl probably uses the same logic to identify each rule and thus needs to be changed, too. Also, IIRC bison allows blanks between the symbol name and the colon, or in other words "generic_set:" is equal to "generic_set :". If this happens after a "missing" semicolon I think your patch does not notice the end of the rule. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [Bug Fix] ECPG: could not use set xxx to default statement
Higuchi-san, > I attached the patch which cope with missing semicolons. > Previous parse.pl find semicolon and dump data to buffer. When > attached patch's parse.pl find new tokens before finding a semicolon, > it also dumps data to buffer. Now this seems to be much easier than I expected. Thanks. My first test show two "missing" semicolons in gram.y. :) Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [Bug Fix] ECPG: could not use some CREATE TABLE AS syntax
Hi Higuchi-san, > I updated and attached the patch. As I show in previous post, this > version is that "IF NOT EXISTS" keyword and variable for "WITH NO > DATA" are added to ecpg.trailer. Thank you, committed. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [PROPOSAL]a new data type 'bytea' for ECPG
Matsumura-san, > I attach a new patch. > ... Thank you so much. This looks very good. Committed to HEAD. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [Bug Fix] ECPG: could not use some CREATE TABLE AS syntax
Hi Higuchi-san,
> My goal is to accept syntax which is currently rejected by ECPG. To
> realize that, I am considering following two ways:
> (a) new syntax of create as statement should be added to ECPG
Correct.
> (b) make ECPG to use not ecpg.trailer but gram.y in the syntax of
> create as statement
I don't see how this would be possible to be honest.
> In (a), we need to keep similar codes in both ecpg.trailer and
> gram.y. Also, if the syntax of create as statement will be changed in
> the future, there is a possibility that it will not be reflected in
> ECPG like this bug. Therefore, I thought that (b) was better and
> created a patch. And, in order to make it the simplest code, some SQL
> which is rejected in current ECPG is accepted in my patch's ECPG.
Yes, I fully understand that. However, in doing so you accept
statements that the backend later on rejects. The sole reason for the
big ecpg grammar is to prevent those cases whenever possible.
> Indeed, CREATE TABLE a AS SELECT * INTO test FROM a; is accepted in
> my patch's ECPG, but the backend always reject, but which SQL should
> be rejected in both ECPG and the backend? Following inappropriate SQL
> are accepted in ECPG but rejected by the backend (I am wondering why
> only CREATE TABLE .. AS .. INTO is rejected and other inappropriate
> SQL are accepted in current ECPG. ).
That does sound like a bug to me. There may cases where it is not
possible to catch an invalid syntax for one reason or another. But I
would definitely go the extra mile to make the parsers as compatible as
possible.
> From the viewpoint of compatibility, if (b) is not good, I will
> consider (a) solution like following:
>
> diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer
> b/src/interfaces/ecpg/preproc/ecpg.trailer
> @@ -34,7 +34,14 @@ CreateAsStmt: CREATE OptTemp TABLE
> create_as_target AS {FoundInto = 0;} SelectSt
> if (FoundInto == 1)
> mmerror(PARSE_ERROR, ET_ERROR,
> "CREATE TABLE AS cannot specify INTO");
>
> - $$ = cat_str(6, mm_strdup("create"), $2,
> mm_strdup("table"), $4, mm_strdup("as"), $7);
> + $$ = cat_str(7, mm_strdup("create"), $2,
> mm_strdup("table"), $4, mm_strdup("as"), $7, $8);
> + }
> +| CREATE OptTemp TABLE IF_P NOT EXISTS
> create_as_target AS {FoundInto = 0;} SelectStmt opt_with_data
> + {
> + if (FoundInto == 1)
> + mmerror(PARSE_ERROR, ET_ERROR, "CREATE
> TABLE AS cannot specify INTO");
> +
> + $$ = cat_str(7, mm_strdup("create"), $2,
> mm_strdup("table if not exists"), $7, mm_strdup("as"), $10, $11);
> }
> ;
>
> I also want to hear your opinion. I will change my opinion flexibly.
I agree that this the way to go.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [Bug Fix] ECPG: could not use some CREATE TABLE AS syntax
Higuchi-san, > I found some "CREATE TABLE ... AS ... " syntaxes could not be used in > ECPG. > ... > [Investigation] > In my investigation, parse.pl ignore type CreateAsStmt of gram.y and > CreateAsStmt is defined in ecpg.trailer. ECPG use ecpg.trailer's > CreateAsStmt. However, ecpg.trailer lacks some syntaxes. Correct, the syntax of create as statement was changed and those changes have not been added to ecpg. > I feel ignoring type CreateAsStmt of gram.y is strange. Seeing > ecpg.trailer, it seems that ECPG wanted to output the message "CREATE > TABLE AS cannot specify INTO" but is this needed now? In view of the > maintenance, ECPG should use not ecpg.trailer's definition but > gram.y's one. I beg to disagree, or I don't understand. Why would ecpg's changes to the statement be wrong nowadays? > I attached the patch for this and I will register this for next CF. I think the patch does not work correctly. The statement CREATE TABLE a AS SELECT * INTO test FROM a; is accepted with your patch, but it is not accepted in current ecpg nor is it accepted by the backend when you execute it through ecpg. The whole change of this rule has been made to make sure this syntax is not accepted. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [PROPOSAL]a new data type 'bytea' for ECPG
Matsumura-san, > I agree that the special route is ugly, but I cannot remove them > completely. > I try to implement Idea-2. In same time, I try to move if(bytea) > blocks to > new function for readability. > > e.g. move the following to new function set_data_attr(). I don't think this will help much, don't bother. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [PROPOSAL]a new data type 'bytea' for ECPG
Matsumura-san,
> Current architecture:
> Internal expression of varchar is C-string that includes length
> information implicitly
> because the length can be computed by strlen().
> ECPGdo(ecpg_build_params) assumes that the data in descriptor is C-
> string encoded.
>
> In other hand, bytea data is binary that doesn't include any length
> information.
> And the merit of my proposal is that bytea data can be sent to
> backend without
> C-string encodeing overhead. They are different points from varchar.
Yes, I agree with this. But it does not explain why we cannot just add
a length parameter. And it neither explains why we need so many if
(!bytea) { thisandthat } else { somethingelse } blocks. I would prefer
the integration to be smoother. Hopefully that is possible.
> My Idea-2 is that:
> - ECPGset_desc copies data to descriptor_item.data, set the length to
> dscriptor_item.data_len and set type information to
> descriptor_item.is_binary.
> - ecpg_build_params only memcpy as folowing without ecpg_store_input:
>
> if (descriptor_item.is_binary)
> memcpy(, descriptor_item.data,
> descriptor_item.data_len)
Isn't that a better way then? This looks more smoothly to me.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [PROPOSAL]a new data type 'bytea' for ECPG
Matsumura-san, > I try to explain as follows. I would like to receive your comment. > ... I'm afraid I don't really understand your explanation. Why is handling a bytea so different from handling a varchar? I can see some differences due to its binary nature, but I do not understand why it needs so much special handling for stuff like its length? There is a length field in the structure but instead of using it the data field is used to store both, the length and the data. What am I missing? Please keep in mind that I did not write the descriptor code, so I may very well not see the obvious. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [PROPOSAL]a new data type 'bytea' for ECPG
Matsumura-san, > I remove it and attach a new patch. Please review it. > I feel sorry for asking you to reveiw without contribution. Don't worry. There is one thing that I don't understand right now. YOu change ecpg_store_input() to handle the bytea data type, yet you also change ECPGset_desc() to not use ecpg_store_input() in case of an bytea. This looks odd to me. Can you please explain that to me? Thanks Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [PROPOSAL]a new data type 'bytea' for ECPG
Matsumura-san, > Sorry to bother you, but I would be grateful if you would comment to me. Sorry, I didn't know you were waiting on a reply by me. > > I have a question about ecpg manual when I add article for bytea. > > I wonder what does the following about VARCHAR mean. > > ... > > I think, if the footnote of VARCHAR is meaningless, I remove it while I add > > the article for bytea. (I didn't remove in this patch.) I have no idea where the footnote comes from, but I agree that it doesn't seem to make sense. The datatype varchar in the C code is handled by the preprocessor and replaced by a struct definition anyway. Feel free to remove. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: Thread-unsafe coding in ecpg
> I like having a hard limit on the number of loop iterations; > that should ensure that the test terminates no matter how confused > ecpglib is. I get your point and thus will only clean up the tests a little bit. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: Thread-unsafe coding in ecpg
> It looks to me like the reason that the ecpg tests went into an > infinite > loop is that compat_informix/test_informix.pgc has not considered the > possibility of repeated statement failures: > ... Correct, this was missing a safeguard. > I know zip about ecpg coding practices, but wouldn't it be a better > idea > to break out of the loop on seeing an error? I wonder if it would be better to make the test cases use the proper whenever command instead. That would give us a slightly better functionality testing I'd say. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: Thread-unsafe coding in ecpg
> Thanks for reviewing! I've pushed this now (to HEAD only for the > moment), > we'll see what the buildfarm thinks. > > BTW, I found another spot in descriptor.c where ecpglib is using > setlocale() for the same purpose. Perhaps that one's not reachable > in threaded apps, but I didn't see any obvious reason to think so, > so I changed it too. Thanks Tom. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: Thread-unsafe coding in ecpg
> > The question is, what do we do on those platforms? Use setlocale() > > or > > fallback to (a) and document that ecpg has to run in a C locale? > > No, we shouldn't use setlocale(), because it clearly is hazardous > even on platforms where it doesn't fail outright. I don't see > anything so wrong with just documenting the hazard. The situation Actually I meant using setlocale() and documenting that it must not be used with threads, or document it must not be used with locales? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: Thread-unsafe coding in ecpg
> While (b) has more theoretical purity, I'm inclined to think it > doesn't really improve anybody's life compared to (a), because > --disable-thread-safety doesn't actually stop anyone from using > libpq or ecpglib in threaded environments. It just makes it > more likely to fail when they do. The question is, what do we do on those platforms? Use setlocale() or fallback to (a) and document that ecpg has to run in a C locale? We could also rewrite the parsing of numbers to not be locale dependent. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: Thread-unsafe coding in ecpg
> So my conclusion is that this version of setlocale() has some
> thread-safety issues. I was all set to go file a bug report
> when I noticed this in the POSIX spec for setlocale:
>
> The setlocale() function need not be thread-safe.
>
> as well as this:
>
> The global locale established using setlocale() shall only be
> used
> in threads for which no current locale has been set using
> uselocale() or whose current locale has been set to the global
> locale using uselocale(LC_GLOBAL_LOCALE).
This one was new to me.
> IOW, not only is setlocale() not necessarily thread-safe itself,
> but using it to change locales in a multithread program is seriously
> unsafe because of concurrent effects on other threads.
Agreed.
> Therefore, it's plain crazy for ecpg to be calling setlocale() inside
> threaded code. It looks to me like what ecpg is doing is trying to
> defend
> itself against non-C LC_NUMERIC settings, which is laudable, but this
> implementation of that is totally unsafe.
>
> Don't know what's the best way out of this. The simplest thing would
> be to just remove that code and document that you'd better run ecpg
> in LC_NUMERIC locale, but it'd be nice if we could do better.
How about attached patch? According to my manpages it should only
affect the calling threat. I only tested it on my own system so far.
Could you please have a look and/or test on other systems?
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
diff --git a/src/interfaces/ecpg/ecpglib/ecpglib_extern.h b/src/interfaces/ecpg/ecpglib/ecpglib_extern.h
index 1c9bce1456..92ff7126d9 100644
--- a/src/interfaces/ecpg/ecpglib/ecpglib_extern.h
+++ b/src/interfaces/ecpg/ecpglib/ecpglib_extern.h
@@ -61,7 +61,8 @@ struct statement
bool questionmarks;
struct variable *inlist;
struct variable *outlist;
- char *oldlocale;
+ locale_t clocale;
+ locale_t oldlocale;
int nparams;
char **paramvalues;
PGresult *results;
diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c
index 3f5034e792..ba03607d5c 100644
--- a/src/interfaces/ecpg/ecpglib/execute.c
+++ b/src/interfaces/ecpg/ecpglib/execute.c
@@ -102,7 +102,8 @@ free_statement(struct statement *stmt)
free_variable(stmt->outlist);
ecpg_free(stmt->command);
ecpg_free(stmt->name);
- ecpg_free(stmt->oldlocale);
+ if (stmt->clocale)
+ freelocale(stmt->clocale);
ecpg_free(stmt);
}
@@ -1773,13 +1774,18 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
* Make sure we do NOT honor the locale for numeric input/output since the
* database wants the standard decimal point
*/
- stmt->oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno);
- if (stmt->oldlocale == NULL)
+ stmt->clocale = newlocale(LC_NUMERIC_MASK, "C", (locale_t) 0);
+ if (stmt->clocale == (locale_t)0)
+ {
+ ecpg_do_epilogue(stmt);
+ return false;
+ }
+ stmt->oldlocale = uselocale(stmt->clocale);
+ if (stmt->oldlocale == (locale_t)0)
{
ecpg_do_epilogue(stmt);
return false;
}
- setlocale(LC_NUMERIC, "C");
#ifdef ENABLE_THREAD_SAFETY
ecpg_pthreads_init();
@@ -1983,7 +1989,7 @@ ecpg_do_epilogue(struct statement *stmt)
return;
if (stmt->oldlocale)
- setlocale(LC_NUMERIC, stmt->oldlocale);
+ uselocale(stmt->oldlocale);
free_statement(stmt);
}
Re: [PROPOSAL]a new data type 'bytea' for ECPG
Matsumura-san,
> > I meaned that existing applications that receive data of bytea
> > column
> > with using sqlda will encounter their unknown type(=ECPG.bytea) in
> > sqlvar_struct.sqltype.
> >
> > You mean if they are not recompiled? If so, yes, how else could
> > that be
> > handled?
>
> Even if they are recompiled, they will fail.
>
> switch (sqlvar_struct.sqltype)
> {
> case ECPG.int: break;
> case ECPG.char: break;
> /* There is no case for ECPG.bytea */
> default: abort();
Sorry, I should have been more precise. I meant if they are not
recompiled against the new ecpglib which has a case for ECPG.bytea.
> There is an idea as following, but it seems to be ugly.
>
> Implement a parameter for ecpglib.
> The parameter means whether application want to receive
> bytea data in binary format or not. Default is "not".
> # I don't know any ecpglib's parameter like it.
>
> In other words, if application uses "bytea" type host variable,
> ecpglib could know its intent, but in case of sqlda ecpglib could
> not know it.
I'm at a loss here. I don't understand what you are trying to say.
An incompatible change is ok if we change the version number of the
library accordingly.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [PROPOSAL]a new data type 'bytea' for ECPG
Matsumura-san, > > I do think, though, we should change the debug output for > > ecpg_free_params(). > > I try to change about it. Next patch will print binary in hex-format. Thank you. > > > > The patch does not support ECPG.bytea in sqltype of "struct > > > > sqlvar_struct" > > > > because of compatibility. > > > > Sorry I do not really understand what you mean. Could you please > > explain? > > I meaned that existing applications that receive data of bytea column > with using sqlda will encounter their unknown type(=ECPG.bytea) in > sqlvar_struct.sqltype. You mean if they are not recompiled? If so, yes, how else could that be handled? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [PROPOSAL]a new data type 'bytea' for ECPG
Matsumura-san, > Sorry to bother you, but I hope any comment of yours. It is no bother. I'm fine with the patch if it does not work against the standard. I do think, though, we should change the debug output for ecpg_free_params(). The way it is now it prints binary values which we also have in our test suite. I'm afraid this will come back to haunt us. > > The patch does not support ECPG.bytea in sqltype of "struct > > sqlvar_struct" > > because of compatibility. Sorry I do not really understand what you mean. Could you please explain? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [PROPOSAL]a new data type 'bytea' for ECPG
Hi Ryo-san, > # Please call me Ryo. Matsumura is too long. Thanks. > In Pro*C, the data should be represented as hex format C string. Just to clarify, there is no special datatype for binary data? > bytea as a type of table definition may correspond to BLOB in the > standard. Would we prefer to add a blob datatype then? > It seems that there is no defact and no product following to the > standards. > I wonder whether bytea should follow to the standard completely or > follow to existing varchar for usability. Do you see any disadvantage of following the standard? I don't really see where the usability drawback is. In general I would prefer being as close to the standard as reasonably possible. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: Intermittent ECPG test failures on Windows buildfarm machines
Am Montag, den 30.04.2018, 00:22 -0400 schrieb Tom Lane: > Observe the following buildfarm failures: > ... > The common feature is that a single ECPG test case emitted an empty > stdout > file. There is no other indication of a problem: the corresponding > stderr > output files are correct (and no, the "correct" contents of those > aren't > empty), the test process exited with status zero, and there's no sign > of > an issue in the postmaster log. And it's a different test case each > time. > > Baffled ... any ideas? AFAICT there were like 4 commits to ecpg in March that were also backported to 9.6. And while some included changes to the test suite I have no idea which, if any, might result in this kind of problem. Also there was at least one change to the Windows build system that impacted ecpg. Is there anyone out there with a Windows system who could bisect the tree and find which commit is the culprit? Or did we have any changes to the buildfarm scripts that may be causing this? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: ECPG oracle mode test program patch
> The attached patch corrects the cursor name. Fixed, thanks for the patch. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: Fix error in ECPG while connection handling
> Now, ideally the connection would have been null here, but, as the > 'ClosePortalStmt' > rule freed the connection but did not set it to NULL, it still sees > that there > is a connection(which is actually having garbage in it) and throws an > error. Thanks for spotting and fixing. I will push the patch as soon as I'm online again. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [HACKERS] datetime.h defines like PM conflict with external libraries
> Michael, is there any problem including datatype/* headers in ecpg > that > are frontend clean? I see no such usage so far, that's why I'm > asking. When the pgtypes library was created we tried to include only the bits and pieces needed to not create unnecessary hassles, but if it compiles cleanly I'm fine either way. I'm assuming you're talking about including the files for compiling ecpg, not as externally visible header files, right? michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [PATCH] ECPG bug fix in preproc when indicator struct is shorter than record struct
> Do you want patches for the back ports as well? > I noticed that between 9.6 (which is what we're using with this > customer) and 10 the variable arrsiz was renamed to arrsize, so > slight differences. Did not check earlier releases yet. Na, don't worry, git cherry-pick and conflict resolution will do the trick. But thanks for the heads-up. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Re: [PATCH] ECPG bug fix in preproc when indicator struct is shorter than record struct
> Attached is a proposed patch to fix a bug in the ECPG preprocessor > that generates application code that core dumps at run-time. When the > input pgc code uses a record struct for returning query results and > uses an indicator struct that has fewer fields than the record > struct, the generated .c code will compile with no warning but core > dump. This situation comes up when a developer adds a field to an > existing query, adds the field to the record struct and forgets to > add the field to the indicator struct. Thanks for spotting and fixing, committed. > The attached sample files are a simple sample of pgc code that can be > used to see the difference in before and after generation and the > before and after generated code. Next time it would be nice if the test case was self-contained. Wasn't that difficult to figure out the table layout, though. :) > If accepted, this bug fix can be back ported to earlier versions of > ecpg as well. As usual this will be done after a couple of days, if no problems appear. I'm pretty sure there won't but sticking to my workflow here. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
